> -----Original Message----- > From: Igor Mammedov [mailto:imamm...@redhat.com] > Sent: Wednesday, April 25, 2018 8:32 AM > To: Michael S. Tsirkin <m...@redhat.com> > Cc: Schmauss, Erik <erik.schma...@intel.com>; qemu-devel@nongnu.org; Xiao > Guangrong <xiaoguangrong.e...@gmail.com>; Williams, Dan J > <dan.j.willi...@intel.com> > Subject: Re: [PATCH] acpi/nvdimm: remove forward name references > > On Wed, 25 Apr 2018 17:17:04 +0300 > "Michael S. Tsirkin" <m...@redhat.com> wrote: > > > On Wed, Apr 25, 2018 at 03:49:38PM +0200, Igor Mammedov wrote: > > > On Tue, 24 Apr 2018 21:06:37 +0300 > > > "Michael S. Tsirkin" <m...@redhat.com> wrote: > > > > > > > On Tue, Apr 24, 2018 at 05:47:58PM +0000, Schmauss, Erik wrote: > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Michael S. Tsirkin [mailto:m...@redhat.com] > > > > > > Sent: Tuesday, April 24, 2018 10:43 AM > > > > > > To: Igor Mammedov <imamm...@redhat.com> > > > > > > Cc: Schmauss, Erik <erik.schma...@intel.com>; > > > > > > qemu-devel@nongnu.org; Xiao Guangrong > > > > > > <xiaoguangrong.e...@gmail.com>; Williams, Dan J > > > > > > <dan.j.willi...@intel.com> > > > > > > Subject: Re: [PATCH] acpi/nvdimm: remove forward name > > > > > > references > > > > > > > > > > > > On Tue, Apr 24, 2018 at 09:57:22AM +0200, Igor Mammedov wrote: > > > > > > > On Tue, 24 Apr 2018 04:02:40 +0300 "Michael S. Tsirkin" > > > > > > > <m...@redhat.com> wrote: > > > > > > > > [...] > > > > > > > From spec > > > > > > > DefOpRegion := OpRegionOp NameString RegionSpace > > > > > > > RegionOffset RegionLen RegionOffset := TermArg => Integer > > > > > > > TermArg := Type2Opcode > > > > > > > | DataObject | ArgObj | LocalObj > > > > > > > > > > > > > > So named object is not accepted, > > > > > > > > > > > > might be worth checking what happens with actual OSPMs. > > > > > > If it does happen to work, we can try tweaking the ACPI spec to > > > > > > allow > this. > > > > > > > > > > I'm looking at the ACPI6.2a spec on page 840 and it says > > > > > TermArg := Type2Opcode | DataObject | Argterm |LocalTerm > |NameString |SymbolicExpression > > > > > > > > Oh right, that's there since ACPI 6.0. > > > Strange, I've rechecked AML definition of TermArg in 6.0 and 6.2a > > > and it still says only > > > > > > TermArg := Type2Opcode | DataObject | ArgObj | LocalObj > > > > > > For 6.2a, I'm looking at chapter "20.2.5 Term Objects Encoding" > > > where exactly do you guys see that longer variant? > > > > Oh interesting. That's in > > 19.2.3 ASL Root and Secondary Terms > > > > How come it's not the same? > bugs are everywhere, see "20.2.5 Term Objects Encoding" > > > > > > > > > > As usual, the issue is that > > > > we can't easily check what does OSPM support. > > > > This is really something worth fixing in the spec IMHO. > > > > We could just try and test a bunch of guest OSPMs and if they > > > > happen to work, switch to that. Lots of work for uncertain > > > > benefit. > > > I think some windows versions weren't happy with it when we were > > > trying to use dynamic operation regions with offset as namestring. > > > > > > Good to know, thanks.
I have made a big mistake. I was comparing the resulting SSDT of this patch to an SSDT Generated by a much older version of kvm/qemu. The latest kvm/qemu upstream builds the SSDT with the operation region contained inside of methods. This should resolve MEMA of the operation region when the method is invoked. The older SSDT that I was looking at declared the memory region outside of methods and had a forward reference to MEMA which we no longer support. So the latest qemu/kvm works just fine and this patch is not needed, Dan has confirmed that the latest qemu/kvm works for his nvdimm setup. Thanks for all of the help and I'm sorry for any confusion that I might have caused! > I'll try to retest MS VMs I have here, to see which one breaks. >
