Re: [Qemu-devel] [PATCH v7 25/35] nvdimm acpi: init the resource used by NVDIMM ACPI

[Igor Mammedov]

On Thu, 5 Nov 2015 18:15:31 +0800
Xiao Guangrong <address@hidden> wrote:

> 
> 
> On 11/05/2015 05:58 PM, Igor Mammedov wrote:
> > On Mon,  2 Nov 2015 17:13:27 +0800
> > Xiao Guangrong <address@hidden> wrote:
> >
> >> A page staring from 0xFF00000 and IO port 0x0a18 - 0xa1b in guest are
> >                                 ^^ missing one 0???
> >
> >> reserved for NVDIMM ACPI emulation, refer to docs/specs/acpi_nvdimm.txt
> >> for detailed design
> >>
> >> A parameter, 'nvdimm-support', is introduced for PIIX4_PM and ICH9-LPC
> >> that controls if nvdimm support is enabled, it is true on default and
> >> it is false on 2.4 and its earlier version to keep compatibility
> >>
> >> Signed-off-by: Xiao Guangrong <address@hidden>
> > [...]
> >
> >> @@ -33,6 +33,15 @@
> >>    */
> >>   #define MIN_NAMESPACE_LABEL_SIZE      (128UL << 10)
> >>
> >> +/*
> >> + * A page staring from 0xFF00000 and IO port 0x0a18 - 0xa1b in guest are
> >                                   ^^^ missing 0 or value in define below 
> > has an extra 0
> >
> >> + * reserved for NVDIMM ACPI emulation, refer to docs/specs/acpi_nvdimm.txt
> >> + * for detailed design.
> >> + */
> >> +#define NVDIMM_ACPI_MEM_BASE          0xFF000000ULL
> > it still maps RAM at arbitrary place,
> > that's the reason why VMGenID patches hasn't been merged for
> > more than several months.
> > I'm not against of using (more exactly I'm for it) direct mapping
> > but we should reach consensus when and how to use it first.
> >
> > I'd wouldn't use addresses below 4G as it may be used firmware or
> > legacy hardware and I won't bet that 0xFF000000ULL won't conflict
> > with anything.
> > An alternative place to allocate reserve from could be high memory.
> > For pc we have "reserved-memory-end" which currently makes sure
> > that hotpluggable memory range isn't used by firmware.
> >
> > How about making API that allows to map additional memory
> > ranges before reserved-memory-end and pushes it up as mappings are
> > added.
> 
> That what i did in the v1/v2 versions, however, as you noticed, using 64-bit
> address in ACPI in QEMU is not a easy work - we can not simply make
> SSDT.rev = 2 to apply 64 bit address, the reason i have documented in
> v3's changelog:
> 
>    3) we figure out a unused memory hole below 4G that is 0xFF00000 ~
>       0xFFF00000, this range is large enough for NVDIMM ACPI as build 64-bit
>       ACPI SSDT/DSDT table will break windows XP.
>       BTW, only make SSDT.rev = 2 can not work since the width is only 
> depended
>       on DSDT.rev based on 19.6.28 DefinitionBlock (Declare Definition Block)
>       in ACPI spec:
> | Note: For compatibility with ACPI versions before ACPI 2.0, the bit
> | width of Integer objects is dependent on the ComplianceRevision of the DSDT.
> | If the ComplianceRevision is less than 2, all integers are restricted to 32
> | bits. Otherwise, full 64-bit integers are used. The version of the DSDT sets
> | the global integer width for all integers, including integers in SSDTs.
>    4) use the lowest ACPI spec version to document AML terms.
> 
> The only way introducing 64 bit address is adding XSDT support that what
> Michael did before, however, it seems it need great efforts to do it as
> it will break OVMF. It's a long term workload. :(
to enable 64-bit integers in AML it's sufficient to change DSDT revision to 2,
I already have a patch that switches DSDT/SSDT to rev2.
Tests show it doesn't break WindowsXP (which is rev1) and uses 64-bit integers
on linux & later Windows versions.

> 
> The luck thing is, the ACPI part is not ABI, we can move it to the high
> memory if QEMU supports XSDT is ready in future development.
But mapped control region is ABI and we can't change it if we find out later
that it breaks something.

> 
> Thanks!