From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48072) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VsfxP-0002dS-Cw for qemu-devel@nongnu.org; Mon, 16 Dec 2013 16:44:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VsfxK-0001sx-Km for qemu-devel@nongnu.org; Mon, 16 Dec 2013 16:44:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33967) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VsfxK-0001sr-CJ for qemu-devel@nongnu.org; Mon, 16 Dec 2013 16:44:02 -0500 Date: Mon, 16 Dec 2013 22:42:49 +0100 From: Igor Mammedov Message-ID: <20131216224249.402e6ab0@thinkpad> In-Reply-To: <20131216195053.GA23900@redhat.com> References: <1385001528-12003-1-git-send-email-imammedo@redhat.com> <1385001528-12003-24-git-send-email-imammedo@redhat.com> <20131121093701.GA3140@redhat.com> <20131125153913.1af63baa@nial.usersys.redhat.com> <20131216195053.GA23900@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 23/27] pc: ACPI BIOS: implement memory hotplug interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: peter.maydell@linaro.org, mdroth@linux.vnet.ibm.com, stefanb@linux.vnet.ibm.com, hutao@cn.fujitsu.com, marcel.a@redhat.com, mjt@tls.msk.ru, armbru@redhat.com, qemu-devel@nongnu.org, vasilis.liaskovitis@profitbricks.com, quintela@redhat.com, chegu_vinod@hp.com, kraxel@redhat.com, aliguori@amazon.com, pbonzini@redhat.com, lcapitulino@redhat.com, stefanha@redhat.com, afaerber@suse.de On Mon, 16 Dec 2013 21:50:53 +0200 "Michael S. Tsirkin" wrote: > On Mon, Nov 25, 2013 at 03:39:13PM +0100, Igor Mammedov wrote: > > On Thu, 21 Nov 2013 11:37:01 +0200 > > "Michael S. Tsirkin" wrote: > > > > > On Thu, Nov 21, 2013 at 03:38:44AM +0100, Igor Mammedov wrote: > > > > - provides static SSDT object for memory hotplug > > > > - SSDT template for memory devices and runtime generator > > > > of them in SSDT table. > > > > > > > > Signed-off-by: Vasilis Liaskovitis > > > > Signed-off-by: Igor Mammedov > > > > --- > > [...] > > > > > > diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl > > > > index a4484b8..b0c581e 100644 > > > > --- a/hw/i386/ssdt-misc.dsl > > > > +++ b/hw/i386/ssdt-misc.dsl > > > > @@ -116,4 +116,183 @@ DefinitionBlock ("ssdt-misc.aml", "SSDT", 0x01, "BXPC", "BXSSDTSUSP", 0x1) > > > > } > > > > } > > > > } > > > > + > > > > + External(MTFY, MethodObj) > > > > + > > > > + Scope(\_SB) { > > > > + Device(MHPD) { > > > > + Name(_HID, EISAID("PNP0C08")) > > > > + > > > > + ACPI_EXTRACT_NAME_WORD_CONST ssdt_mctrl_port > > > > + Name(MHPP, 0xFFFF) > > > > + > > > > + ACPI_EXTRACT_NAME_DWORD_CONST ssdt_mctrl_nr_slots > > > > + Name(MDNR, 0x12345678) > > > > + > > > > + /* Memory hotplug IO registers */ > > > > + OperationRegion(HPMR, SystemIO, MHPP, 24) > > > > + > > > > + Method(_CRS, 0, Serialized) { > > > > + Name(CRS, ResourceTemplate() { > > > > + IO(Decode16, 0x00, 0x00, 0x01, 24, IO) > > > > + }) > > > > > > > > > Declaring name makes us serialize it. > > > Can't we use a local variable? > > If that works, I'll change it. > > > > But I have a question of my own perhaps to Paolo or Gerd, > > Do we really need this _CRS, because if you look at next hunk > > _STA should report not present but functioning to avoid windows BSOD. > > So there is not guaranties that OSPM would care or even query it > > and honor _CRS provided range. > > > > Yes this worries me too. > Making _STA non present looks wrong. I've made status as not present and enabled/decoding and functioning, but that doesn't mean that OSPM respects its _CRS. > I suspect we need to look for some other way to make > windows not crash that will make it respect the _CRS. > Also which windows? All of them? I've tested with WS2012DCx64, PV panic doesn't crash windows because it's showed as Unknown device, which also doesn't guaranty that windows would respects it's _CRS. OSPM already knows that range is used by OperationRegion. Question is do we really need to expose OperationRegion as _CRS? > > > > [...] > > > > + > > > > + Method(_STA, 0) { > > > > + If (LEqual(MDNR, Zero)) { > > > > + Return(0x0) > > > > + } > > > > + /* Leave bit 0 cleared to avoid Windows BSOD */ > > > > + Return(0xA) > > > > + } > > [...] -- Regards, Igor