qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Xiao Guangrong
Subject: Re: [Qemu-devel] [PATCH v7 25/35] nvdimm acpi: init the resource used by NVDIMM ACPI
Date: Fri, 6 Nov 2015 16:56:10 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0



On 11/06/2015 04:31 PM, Xiao Guangrong wrote:


On 11/05/2015 10:49 PM, Igor Mammedov wrote:
On Thu, 5 Nov 2015 21:33:39 +0800
Xiao Guangrong <address@hidden> wrote:



On 11/05/2015 09:03 PM, Igor Mammedov wrote:
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.

Great, i remembered i did the similar test (directly change DSDT to rev2) and it
caused winXP blue screen. Could you please tell me where i can find your patch?
https://github.com/imammedo/qemu/commits/mhpt_table_v2
following changes revision:
  pc: acpi: bump DSDT/SSDT compliance revision to v2
and here is user:
  acpi: memhp: simplify MCRS() using 64-bit math

when writing ASL one shall make sure that only XP supported
features are in global scope, which is evaluated when tables
are loaded and features of rev2 and higher are inside methods.
That way XP doesn't crash as far as it doesn't evaluate unsupported
opcode and one can guard those opcodes checking _REV object if neccesary.


Really a good study case to me, i tried your patch and moved the 64 bit
staffs to the private method, it worked. :)

Igor, is this the API you want?

BTW, after move the control region to 4G above, is it acceptable to reserve 
enough
memory containing whole FIT and dsm memory as we did it in previous version?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]