From patchwork Fri Mar 28 17:52:38 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 27369 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-qg0-f70.google.com (mail-qg0-f70.google.com [209.85.192.70]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 09DE220062 for ; Fri, 28 Mar 2014 17:55:19 +0000 (UTC) Received: by mail-qg0-f70.google.com with SMTP id z60sf6308923qgd.1 for ; Fri, 28 Mar 2014 10:55:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:in-reply-to:message-id :references:user-agent:mime-version:cc:subject:precedence:list-id :list-unsubscribe:list-post:list-help:list-subscribe:sender :errors-to:x-original-sender:x-original-authentication-results :mailing-list:list-archive:content-type:content-transfer-encoding; bh=wUp5obGbkr3dCXS/Jk76lIAXv7gFpU8bKhOOnDJVEE8=; b=P060KBZwAipsQXsJKUlCLBavPLndiPJ9oCDJUqOMct7LI0VcS01qbWeTgpoLlFDZTQ 5nU/0knHo/YGFBAZAk3l7mCefd+a3TT7nb+C92Yj5yUP+QjbOiMpl3+noQM4Ocrr2WsK RcXlhmKlvoIXou5Jz7jC2fqPnXZj8QPlcYbi0CHuHXOZbmiXc/HyioM2u5ETHcZlF3aF MLRQKDngvYUp7hnAyDx5eEU0vwLon72vAmOApKFBoQge+CdF348KjtQIXmVbyH+20lD1 f20Vs6txsBI9ExKOKTJefcOT2eLPaPxoDt6K+YbpmcUIkFED8tJaaqX0bUtwKEp8BDaO EBRg== X-Gm-Message-State: ALoCoQkGPzV/kJSBe7m+ftUAO5p8m0ywcL44JSRTGQz9wFzIj4FJ7KRt/o41MNTspmHvK4gYXKam X-Received: by 10.58.195.202 with SMTP id ig10mr2577454vec.38.1396029319721; Fri, 28 Mar 2014 10:55:19 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.22.75 with SMTP id 69ls1687454qgm.94.gmail; Fri, 28 Mar 2014 10:55:19 -0700 (PDT) X-Received: by 10.220.92.135 with SMTP id r7mr8310930vcm.11.1396029319551; Fri, 28 Mar 2014 10:55:19 -0700 (PDT) Received: from mail-ve0-f180.google.com (mail-ve0-f180.google.com [209.85.128.180]) by mx.google.com with ESMTPS id sh5si1366598vdc.158.2014.03.28.10.55.19 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 28 Mar 2014 10:55:19 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.128.180 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.128.180; Received: by mail-ve0-f180.google.com with SMTP id jz11so6145116veb.11 for ; Fri, 28 Mar 2014 10:55:19 -0700 (PDT) X-Received: by 10.220.162.6 with SMTP id t6mr8421844vcx.12.1396029319457; Fri, 28 Mar 2014 10:55:19 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.220.12.8 with SMTP id v8csp27991vcv; Fri, 28 Mar 2014 10:55:19 -0700 (PDT) X-Received: by 10.58.188.52 with SMTP id fx20mr1442325vec.28.1396029319107; Fri, 28 Mar 2014 10:55:19 -0700 (PDT) Received: from lists.xen.org (lists.xen.org. [50.57.142.19]) by mx.google.com with ESMTPS id cb3si1371769vdc.113.2014.03.28.10.55.18 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Fri, 28 Mar 2014 10:55:19 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of xen-devel-bounces@lists.xen.org designates 50.57.142.19 as permitted sender) client-ip=50.57.142.19; Received: from localhost ([127.0.0.1] helo=lists.xen.org) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WTay1-0007lR-Ub; Fri, 28 Mar 2014 17:53:21 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WTay0-0007lM-Lu for xen-devel@lists.xenproject.org; Fri, 28 Mar 2014 17:53:20 +0000 Received: from [85.158.139.211:52659] by server-9.bemta-5.messagelabs.com id CA/6B-04350-017B5335; Fri, 28 Mar 2014 17:53:20 +0000 X-Env-Sender: Stefano.Stabellini@citrix.com X-Msg-Ref: server-5.tower-206.messagelabs.com!1396029197!1131271!1 X-Originating-IP: [66.165.176.89] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni44OSA9PiAyMDMwMDc=\n X-StarScan-Received: X-StarScan-Version: 6.11.1; banners=-,-,- X-VirusChecked: Checked Received: (qmail 24870 invoked from network); 28 Mar 2014 17:53:18 -0000 Received: from smtp.citrix.com (HELO SMTP.CITRIX.COM) (66.165.176.89) by server-5.tower-206.messagelabs.com with RC4-SHA encrypted SMTP; 28 Mar 2014 17:53:18 -0000 X-IronPort-AV: E=Sophos;i="4.97,752,1389744000"; d="scan'208";a="116040580" Received: from accessns.citrite.net (HELO FTLPEX01CL01.citrite.net) ([10.9.154.239]) by FTLPIPO01.CITRIX.COM with ESMTP; 28 Mar 2014 17:53:17 +0000 Received: from ukmail1.uk.xensource.com (10.80.16.128) by smtprelay.citrix.com (10.13.107.78) with Microsoft SMTP Server id 14.2.342.4; Fri, 28 Mar 2014 13:53:16 -0400 Received: from kaball.uk.xensource.com ([10.80.2.59]) by ukmail1.uk.xensource.com with esmtp (Exim 4.69) (envelope-from ) id 1WTaxr-0000Ib-2a; Fri, 28 Mar 2014 17:53:11 +0000 Date: Fri, 28 Mar 2014 17:52:38 +0000 From: Stefano Stabellini X-X-Sender: sstabellini@kaball.uk.xensource.com To: Stefano Stabellini In-Reply-To: Message-ID: References: <5335376102000078000033DA@nat28.tlf.novell.com> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 X-DLP: MIA1 Cc: anthony.perard@citrix.com, xen-devel , Stefano Stabellini , Jan Beulich , Paolo Bonzini Subject: Re: [Xen-devel] gross qemu behavior X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: List-Unsubscribe: , List-Post: , List-Help: , List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: stefano.stabellini@eu.citrix.com X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.128.180 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Archive: On Fri, 28 Mar 2014, Stefano Stabellini wrote: > CC'ing Paolo, hoping that he has a better idea on how to solve this > problem. > > > On Fri, 28 Mar 2014, Jan Beulich wrote: > > Hi, > > > > so while doing all that EPT work I naturally also happened to look more > > closely at the EPT table dumps, spotting an odd range of 16 pages > > outside any supposedly populated address range. This range only > > exists when guest memory doesn't extend past (by default) 0xf0000000 > > (the start of MMIO, i.e. normally the frame buffer). After spending quite > > a bit of time I finally figured that this must be a left over of the Cirrus > > VGA ROM, and I would have thought that this > > > > --- a/hw/pci/pci.c > > +++ b/hw/pci/pci.c > > @@ -1976,6 +1976,9 @@ static int pci_add_option_rom(PCIDevice > > } > > > > pci_register_bar(pdev, PCI_ROM_SLOT, 0, &pdev->rom); > > + memory_region_add_subregion_overlap(pdev->bus->address_space_mem, > > + pdev->rom.ram_addr, &pdev->rom, 1); > > + memory_region_del_subregion(pdev->bus->address_space_mem, &pdev->rom); > > > > return 0; > > } > > > > should fix it. It does appear to work as far generic qemu is concerned, > > but once looking at the Xen backend I had to conclude that this just > > can't work: For one, xen_add_to_physmap() and > > xen_remove_from_physmap() are _documented_ (in a comment) to > > only be capable of a single region (VRAM). And the latter - even worse - > > is implemented with a call to xc_domain_add_to_physmap(), completely > > contrary to its name. > > xen_add_to_physmap and xen_remove_from_physmap are just to deal with the > VRAM in their current implementation. > > > > Instrumenting xen_region_{add,del}(), I can see that all regions get > > properly reported to the Xen backend, just that it doesn't handle them > > (this is with above patch in place): > > > > xra(fee00000,100000) > > xra(fec00000,1000) > > xra(fed00000,400) > > xra(80000000,10000) > > xrd(80000000,10000) > > xra(f0000000,800000) > > xra(f1000000,400000) > > xra(f2000000,1000000) > > xra(f3010000,4000) > > xra(f3014000,1000) > > xra(f3015000,3000) > > xra(f3018000,1000) > > xra(f3000000,10000) > > xrd(f3000000,10000) > > xrd(f0000000,800000) > > xra(f0000000,800000) > > mapping vram to f0000000 - f0800000 > > > > Having wasted enough time getting to this point, I'd like to ask you > > to advise a proper fix for this. We definitely shouldn't be leaving > > stuff sitting at arbitrary positions in the physical address space of > > the guest. And the fact that the range gets removed (from Xen's > > perspective, but not from qemu's) when RAM extends beyond > > 0xf0000000 (due to it being replaced with what is actually > > intended to be there) makes me wonder what would happen if the > > ROM got enabled by the guest. > > This is a thorny issue, fixing this behavior is not going to be trivial: > > - The hypervisor/libxc does not currently expose a > xc_domain_remove_from_physmap function. > > - QEMU works by allocating memory regions at the end of the guest > physmap and then moving them at the right place. > > - QEMU can destroy a memory region and in that case we could free the > memory and remove it from the physmap, however that is NOT what QEMU > does with the vga ROM. In that case it calls > memory_region_del_subregion, so we can't be sure that the ROM won't be > mapped again, therefore we cannot free it. We need to move it > somewhere else, hence the problem. > > > But fortunately we don't actually need to add the VGA ROM to the guest > physmap for it to work, QEMU can trap and emulate. In fact even today we > are not mapping it at the right place anyway, see xen_set_memory: > > if (add) { > if (!memory_region_is_rom(section->mr)) { > xen_add_to_physmap(state, start_addr, size, > section->mr, section->offset_within_region); > } else { > > > So the only solution I can see right now is: > > - avoid allocating guest memory for the VGA ROM > That means that at the beginning of xen_ram_alloc we need to realize > that the memory region we are dealing with is the VGA ROM memory region > and avoid calling xc_domain_populate_physmap_exact for it. > > - call g_malloc instead > Simply use g_malloc to allocate QEMU memory for the VGA ROM, > keep track of the allocation in a data structure internal to xen-all.c. > > - make sure that qemu_get_ram_ptr can deal with the different allocation > Now that the VGA ROM is QEMU memory, we need to make sure that > qemu_get_ram_ptr returns the right pointer for it. > > > This is all very fiddly and hackish, but I can't see a better way of > solving the issue. Given that I feel that the explanation is not very clear, I am appending a proof of concept patch. It is obviously horrible, I am by no means suggesting it should be applied. diff --git a/exec.c b/exec.c index 91513c6..bdecc70 100644 --- a/exec.c +++ b/exec.c @@ -1453,6 +1453,7 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length) It should not be used for general purpose DMA. Use cpu_physical_memory_map/cpu_physical_memory_rw instead. */ +extern uint8_t* vga_rom; void *qemu_get_ram_ptr(ram_addr_t addr) { RAMBlock *block = qemu_get_ram_block(addr); @@ -1462,7 +1463,9 @@ void *qemu_get_ram_ptr(ram_addr_t addr) * because we don't want to map the entire memory in QEMU. * In that case just map until the end of the page. */ - if (block->offset == 0) { + if (!strcmp(block->mr->name,"cirrus_vga.rom")) { + return vga_rom; + } else if (block->offset == 0) { return xen_map_cache(addr, 0, 0); } else if (block->host == NULL) { block->host = diff --git a/xen-all.c b/xen-all.c index ba34739..6211946 100644 --- a/xen-all.c +++ b/xen-all.c @@ -101,6 +101,8 @@ typedef struct XenIOState { Notifier wakeup; } XenIOState; +uint8_t* vga_rom; + /* Xen specific function for piix pci */ int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num) @@ -217,6 +219,11 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr) return; } + if (!strcmp(mr->name,"cirrus_vga.rom")) { + vga_rom = g_malloc(size); + return; + } + trace_xen_ram_alloc(ram_addr, size); nr_pfn = size >> TARGET_PAGE_BITS;