From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42125) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VPfp3-0004l0-Rz for qemu-devel@nongnu.org; Fri, 27 Sep 2013 17:43:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VPfow-0006s1-Vi for qemu-devel@nongnu.org; Fri, 27 Sep 2013 17:43:37 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:22638) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VPfow-0006rq-Oz for qemu-devel@nongnu.org; Fri, 27 Sep 2013 17:43:30 -0400 Date: Fri, 27 Sep 2013 17:43:13 -0400 From: Konrad Rzeszutek Wilk Message-ID: <20130927214313.GB21803@phenom.dumpdata.com> References: <33183CC9F5247A488A2544077AF190208159B6E8@SZXEMA503-MBS.china.huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <33183CC9F5247A488A2544077AF190208159B6E8@SZXEMA503-MBS.china.huawei.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Xen-devel] Hvmloader: Add _STA for PCI hotplug slots List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Gonglei (Arei)" , anthony.perard@citrix.com, Stefano Stabellini Cc: "Hanweidong (Randy)" , Yanqiangjun , Luonengjun , "qemu-devel@nongnu.org" , "xen-devel@lists.xen.org" , "Gaowei (UVP)" , "Huangweidong (Hardware)" On Fri, Sep 27, 2013 at 06:29:20AM +0000, Gonglei (Arei) wrote: > Hi, Hey, (CCing Stefano and Anthony). >=20 > In Xen platform, after using upstream qemu, the all of pci devices will= show hotplug in the windows guest.=20 > In this situation, the windows guest may occur blue screen when VM' use= r click the icon of VGA card for trying unplug VGA card. > However, we don't hope VM's user can do such dangerous operation, and s= howing all pci devices inside the guest OS is unfriendly. >=20 > In addition, I find the traditional qemu have not this problem, and KVM= also. >=20 > On the KVM platform, the seabios will read the RMV bits of pci slot (ac= cording the 0xae08 I/O port register),=20 > then modify the SSDT table.=20 >=20 > The key steps as follows: > In Seabios: > #define PCI_RMV_BASE 0xae0c // 0xae08 I/O port register > static void* build_ssdt(void) > { > ... > // build Device object for each slot > u32 rmvc_pcrm =3D inl(PCI_RMV_BASE); > ... > } >=20 > In upstream Qemu, read 0xae0c I/O port register function: > static uint64_t pci_read(void *opaque, hwaddr addr, unsigned int size) > {=20 > ... =20 > case PCI_RMV_BASE - PCI_HOTPLUG_ADDR: > val =3D s->pci0_hotplug_enable; > break; > }=09 > s->pci0_hotplug_enable is set by the follow function: >=20 > static void piix4_update_hotplug(PIIX4PMState *s) > { > ... > s->pci0_hotplug_enable =3D ~0; > s->pci0_slot_device_present =3D 0; >=20 > QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) { > DeviceState *qdev =3D kid->child; > PCIDevice *pdev =3D PCI_DEVICE(qdev); > PCIDeviceClass *pc =3D PCI_DEVICE_GET_CLASS(pdev); > int slot =3D PCI_SLOT(pdev->devfn); > =09 > //setting by PCIDeviceClass *k->no_hotplug > if (pc->no_hotplug) { > s->pci0_hotplug_enable &=3D ~(1U << slot); > } >=20 > s->pci0_slot_device_present |=3D (1U << slot); > } > } >=20 > But, on the XEN platform, ACPI DSDT tables is produced by the hvmloader= ,=20 > more details in this patch=EF=BC=9A > http://xen.1045712.n5.nabble.com/xen-unstable-hvmloader-acpi-dsdt-Fix-P= CI-hotplug-with-the-new-qemu-xen-td4947152.html >=20 > # Node ID 1a912ce93b506a185b54fd97986214e6eff8a0bc=20 > # Parent 6bc03e22f921aadfa7e5cebe92100cb01377947d=20 > hvmloader/acpi/dsdt: Fix PCI hotplug with the new qemu-xen.=20 oddly enough you did not CC the author of said patch? I am doing that for you. >=20 > The ACPI PIIX4 device in QEMU upstream as not the same behavior to=20 > handle PCI hotplug. This patch introduce the necessary change to the=20 > DSDT ACPI table to behave as expceted by the new QEMU.=20 >=20 > To switch to this new DSDT table version, there is a new option=20 > --dm-version to mk_dsdt.=20 >=20 > Change are inspired by SeaBIOS DSDT source code.=20 >=20 > There is few things missing with the new QEMU:=20 > - QEMU provide the plugged/unplugged status only per slot (and not=20 > per func like qemu-xen-traditionnal.=20 > - I did not include the _STA ACPI method that give the status of a=20 > device (present, functionning properly) because qemu-xen does not=20 > handle it.=20 > - I did not include the _RMV method that say if the device can be=20 > removed,=20 > because the IO port of QEMU that give this status always return=20 > true. In=20 > SeaBIOS table, they have a specific _RMV method for VGA, ISA that=20 > return=20 > false. But I'm not sure that we can do the same in Xen. > =09 >=20 > now, I add the _STA method, return the different value according the 0x= ae08 I/O port register on read, > a pci device allow hotplug return 0x1f, a pci device don't allow return= 0x1e. > Then the pci devices which don't allow hotplug will not show inside the= guest OS. So you are saying that the 'the IO port of QEMU that give this status alw= ays return true. " is no longer correct? >=20 > Index: tools/firmware/hvmloader/acpi/mk_dsdt.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- tools/firmware/hvmloader/acpi/mk_dsdt.c (revision 1105) > +++ tools/firmware/hvmloader/acpi/mk_dsdt.c (working copy) > @@ -437,6 +437,10 @@ > indent(); printf("B0EJ, 32,\n"); > pop_block(); > =20 > + stmt("OperationRegion", "SRMV, SystemIO, 0xae0c, 0x04"); > + push_block("Field", "SRMV, DWordAcc, NoLock, WriteAsZeros"); > + indent(); printf("RMV, 32,\n"); > + pop_block(); =20 > /* hotplug_slot */ > for (slot =3D 1; slot <=3D 31; slot++) { > push_block("Device", "S%i", slot); { > @@ -445,6 +449,14 @@ > stmt("Store", "ShiftLeft(1, %#06x), B0EJ", slot); > stmt("Return", "0x0"); > } pop_block(); > + push_block("Method", "_STA, 0");{ > + push_block("If", "And(RMV, ShiftLeft(1, %#06x))", s= lot); > + stmt("Return", "0x1F"); > + pop_block(); > + push_block("Else", NULL); > + stmt("Return", "0x1E"); > + pop_block(); > + };pop_block(); > stmt("Name", "_SUN, %i", slot); > } pop_block(); > } >=20 > Have you any ideas=EF=BC=9FExpecting your reply, thanks in advance! >=20 > Best regards, > -Gonglei > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel