From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [SeaBIOS PATCH v2] hotplug: Add device per func in ACPI DSDT tables Date: Wed, 21 Sep 2011 14:09:08 +0300 Message-ID: <20110921110908.GC16295@redhat.com> References: <20110921014832.GA4597@morn.localdomain> <327a4986-9722-472c-aa01-5e3f72b05763@zmail05.collab.prod.int.phx2.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, jasowang@redhat.com, seabios@seabios.org, alex williamson To: Amos Kong Return-path: Content-Disposition: inline In-Reply-To: <327a4986-9722-472c-aa01-5e3f72b05763@zmail05.collab.prod.int.phx2.redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: seabios-bounces@seabios.org Sender: seabios-bounces@seabios.org List-Id: kvm.vger.kernel.org On Wed, Sep 21, 2011 at 01:39:22AM -0400, Amos Kong wrote: > ----- Original Message ----- > > On Tue, Sep 20, 2011 at 06:45:57AM -0400, Amos Kong wrote: > > > From 48ea1c9188334b89a60b4f9e853e86fc04fda4a5 Mon Sep 17 00:00:00 > > > 2001 > > > From: Amos Kong > > > Date: Tue, 20 Sep 2011 15:38:43 +0800 > > > Subject: [SeaBIOS PATCH v2] hotplug: Add device per func in ACPI > > > DSDT tables > > > > > > Only func 0 is registered to guest driver (we can > > > only found func 0 in slot->funcs list of driver), > > > the other functions could not be cleaned when > > > hot-removing the whole slot. This patch adds > > > device per function in ACPI DSDT tables. > > > > > > Have tested with linux/winxp/win7, hot-adding/hot-remving, > > > single/multiple function device, they are all fine. > > > --- > > > Changes from v1: > > > - cleanup the macros, bios.bin gets back to 128K > > > - notify only when func0 is added and removed > > > > How about moving code into functions so that it isn't duplicated for > > each PCI device. See the patch below as an example (100% untested). Hmm, I sent patches that did a similar thing but in a slightly more compact way. Message ids: 20110919092932.GB4501@redhat.com 20110919093644.GC4501@redhat.com 20110919100434.GA6764@redhat.com Did they not reach you or something's wrong with them? > > Tested, it works as my original patch-v2. > > > The CPU hotplug stuff works this way, except it run-time generates > > the > > equivalent of "Device(S???)" and "PCNT". (It may make sense to > > run-time generate the PCI hotplug as well - it consumes about 10K of > > space to statically generate the 31 devices.) > > I'm ok with the new version. > > Acked-by: Amos Kong > > > -Kevin > > > > > > diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl > > index 08412e2..31ac5eb 100644 > > --- a/src/acpi-dsdt.dsl > > +++ b/src/acpi-dsdt.dsl > > @@ -128,48 +128,6 @@ DefinitionBlock ( > > PCRM, 32, > > } > > > > -#define hotplug_slot(name, nr) \ > > - Device (S##name) { \ > > - Name (_ADR, nr##0000) \ > > - Method (_EJ0,1) { \ > > - Store(ShiftLeft(1, nr), B0EJ) \ > > - Return (0x0) \ > > - } \ > > - Name (_SUN, name) \ > > - } > > - > > - hotplug_slot(1, 0x0001) > > - hotplug_slot(2, 0x0002) > > - hotplug_slot(3, 0x0003) > > - hotplug_slot(4, 0x0004) > > - hotplug_slot(5, 0x0005) > > - hotplug_slot(6, 0x0006) > > - hotplug_slot(7, 0x0007) > > - hotplug_slot(8, 0x0008) > > - hotplug_slot(9, 0x0009) > > - hotplug_slot(10, 0x000a) > > - hotplug_slot(11, 0x000b) > > - hotplug_slot(12, 0x000c) > > - hotplug_slot(13, 0x000d) > > - hotplug_slot(14, 0x000e) > > - hotplug_slot(15, 0x000f) > > - hotplug_slot(16, 0x0010) > > - hotplug_slot(17, 0x0011) > > - hotplug_slot(18, 0x0012) > > - hotplug_slot(19, 0x0013) > > - hotplug_slot(20, 0x0014) > > - hotplug_slot(21, 0x0015) > > - hotplug_slot(22, 0x0016) > > - hotplug_slot(23, 0x0017) > > - hotplug_slot(24, 0x0018) > > - hotplug_slot(25, 0x0019) > > - hotplug_slot(26, 0x001a) > > - hotplug_slot(27, 0x001b) > > - hotplug_slot(28, 0x001c) > > - hotplug_slot(29, 0x001d) > > - hotplug_slot(30, 0x001e) > > - hotplug_slot(31, 0x001f) > > - > > Name (_CRS, ResourceTemplate () > > { > > WordBusNumber (ResourceProducer, MinFixed, MaxFixed, > > PosDecode, > > @@ -762,6 +720,119 @@ DefinitionBlock ( > > Zero /* reserved */ > > }) > > > > + /* PCI hotplug */ > > + Scope(\_SB.PCI0) { > > + /* Methods called by bulk generated PCI devices below */ > > + Method (PCEJ, 1, NotSerialized) { > > + // _EJ0 method - eject callback > > + Store(ShiftLeft(1, Arg0), B0EJ) > > + Return (0x0) > > + } > > + > > + /* Bulk generated PCI hotplug devices */ > > +#define hotplug_func(nr, fn) \ > > + Device (S##nr##fn) { \ > > + Name (_ADR, 0x##nr##000##fn) \ > > + Method (_EJ0, 1) { Return(PCEJ(0x##nr)) } \ > > + Name (_SUN, 0x##nr) \ > > + } The fundamental question is still why would we have _EJ0 methods in functions >0 when they are not individually hotpluggable. I think only function 0 should have _EJ0. > > + > > +#define hotplug_slot(nr) \ > > + hotplug_func(nr, 0) \ > > + hotplug_func(nr, 1) \ > > + hotplug_func(nr, 2) \ > > + hotplug_func(nr, 3) \ > > + hotplug_func(nr, 4) \ > > + hotplug_func(nr, 5) \ > > + hotplug_func(nr, 6) \ > > + hotplug_func(nr, 7) > > + > > + hotplug_slot(01) > > + hotplug_slot(02) > > + hotplug_slot(03) > > + hotplug_slot(04) > > + hotplug_slot(05) > > + hotplug_slot(06) > > + hotplug_slot(07) > > + hotplug_slot(08) > > + hotplug_slot(09) > > + hotplug_slot(0a) > > + hotplug_slot(0b) > > + hotplug_slot(0c) > > + hotplug_slot(0d) > > + hotplug_slot(0e) > > + hotplug_slot(0f) > > + hotplug_slot(10) > > + hotplug_slot(11) > > + hotplug_slot(12) > > + hotplug_slot(13) > > + hotplug_slot(14) > > + hotplug_slot(15) > > + hotplug_slot(16) > > + hotplug_slot(17) > > + hotplug_slot(18) > > + hotplug_slot(19) > > + hotplug_slot(1a) > > + hotplug_slot(1b) > > + hotplug_slot(1c) > > + hotplug_slot(1d) > > + hotplug_slot(1e) > > + hotplug_slot(1f) > > + > > + /* PCI hotplug notify method */ > > + Method(PCNF, 0) { > > + // Local0 = iterator > > + Store (Zero, Local0) > > + While (LLess(Local0, 31)) { > > + Increment(Local0) > > + If (And(PCIU, ShiftLeft(1, Local0))) { > > + PCNT(Local0, 1) > > + } > > + If (And(PCID, ShiftLeft(1, Local0))) { > > + PCNT(Local0, 3) > > + } > > + } > > + Return(One) > > + } > > + > > +#define hotplug_notify(nr) \ > > + If (LEqual(Arg0, 0x##nr)) {Notify(S##nr##0, Arg1)} > > + > > + Method(PCNT, 2) { > > + hotplug_notify(01) > > + hotplug_notify(02) > > + hotplug_notify(03) > > + hotplug_notify(04) > > + hotplug_notify(05) > > + hotplug_notify(06) > > + hotplug_notify(07) > > + hotplug_notify(08) > > + hotplug_notify(09) > > + hotplug_notify(0a) > > + hotplug_notify(0b) > > + hotplug_notify(0c) > > + hotplug_notify(0d) > > + hotplug_notify(0e) > > + hotplug_notify(0f) > > + hotplug_notify(10) > > + hotplug_notify(11) > > + hotplug_notify(12) > > + hotplug_notify(13) > > + hotplug_notify(14) > > + hotplug_notify(15) > > + hotplug_notify(16) > > + hotplug_notify(17) > > + hotplug_notify(18) > > + hotplug_notify(19) > > + hotplug_notify(1a) > > + hotplug_notify(1b) > > + hotplug_notify(1c) > > + hotplug_notify(1d) > > + hotplug_notify(1e) > > + hotplug_notify(1f) > > + } > > + } > > + > > /* CPU hotplug */ > > Scope(\_SB) { > > /* Objects filled in by run-time generated SSDT */ > > @@ -842,49 +913,9 @@ DefinitionBlock ( > > Return(0x01) > > } > > > > -#define gen_pci_hotplug(nr) \ > > - If (And(\_SB.PCI0.PCIU, ShiftLeft(1, nr))) { \ > > - Notify(\_SB.PCI0.S##nr, 1) \ > > - } \ > > - If (And(\_SB.PCI0.PCID, ShiftLeft(1, nr))) { \ > > - Notify(\_SB.PCI0.S##nr, 3) \ > > - } > > - > > Method(_L01) { > > - gen_pci_hotplug(1) > > - gen_pci_hotplug(2) > > - gen_pci_hotplug(3) > > - gen_pci_hotplug(4) > > - gen_pci_hotplug(5) > > - gen_pci_hotplug(6) > > - gen_pci_hotplug(7) > > - gen_pci_hotplug(8) > > - gen_pci_hotplug(9) > > - gen_pci_hotplug(10) > > - gen_pci_hotplug(11) > > - gen_pci_hotplug(12) > > - gen_pci_hotplug(13) > > - gen_pci_hotplug(14) > > - gen_pci_hotplug(15) > > - gen_pci_hotplug(16) > > - gen_pci_hotplug(17) > > - gen_pci_hotplug(18) > > - gen_pci_hotplug(19) > > - gen_pci_hotplug(20) > > - gen_pci_hotplug(21) > > - gen_pci_hotplug(22) > > - gen_pci_hotplug(23) > > - gen_pci_hotplug(24) > > - gen_pci_hotplug(25) > > - gen_pci_hotplug(26) > > - gen_pci_hotplug(27) > > - gen_pci_hotplug(28) > > - gen_pci_hotplug(29) > > - gen_pci_hotplug(30) > > - gen_pci_hotplug(31) > > - > > - Return (0x01) > > - > > + // PCI hotplug event > > + Return(\_SB.PCI0.PCNF()) > > } > > Method(_L02) { > > // CPU hotplug event > >