On 11/03/2016 07:58 PM, Igor Mammedov wrote:
On Thu, 3 Nov 2016 11:51:29 +0800
Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx> wrote:
_FIT is required for hotplug support, guest will inquire the updated
device info from it if a hotplug event is received
s/_FIT/_FIT method/
the same applies to subj. line
Okay.
As FIT buffer is not completely mapped into guest address space, so a
new function, Read FIT whose UUID is UUID
648B9CF2-CDA1-4312-8AD9-49C4AF32BD62, handle 0x10000, function index
is 0x1, is reserved by QEMU to read the piece of FIT buffer. The buffer
is concatenated before _FIT return
Commit message hard to read/parse, it might be better if I'd use simple
short sentences.
Okay, will change it to:
As FIT buffer is not completely mapped into guest address space, Read_FIT
method is introduced to read NFIT structures blob from QEMU, The buffer
is concatenated before _FIT return
Refer to docs/specs/acpi-nvdimm.txt for detailed design
Signed-off-by: Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx>
---
docs/specs/acpi_nvdimm.txt | 63 ++++++++++++-
hw/acpi/nvdimm.c | 225 ++++++++++++++++++++++++++++++++++++++++++---
2 files changed, 271 insertions(+), 17 deletions(-)
diff --git a/docs/specs/acpi_nvdimm.txt b/docs/specs/acpi_nvdimm.txt
index 0fdd251..364e832 100644
--- a/docs/specs/acpi_nvdimm.txt
+++ b/docs/specs/acpi_nvdimm.txt
@@ -65,8 +65,8 @@ _FIT(Firmware Interface Table)
The detailed definition of the structure can be found at ACPI 6.0: 5.2.25
NVDIMM Firmware Interface Table (NFIT).
-QEMU NVDIMM Implemention
-========================
+QEMU NVDIMM Implementation
+==========================
QEMU uses 4 bytes IO Port starting from 0x0a18 and a RAM-based memory page
for NVDIMM ACPI.
@@ -82,6 +82,16 @@ Memory:
ACPI writes _DSM Input Data (based on the offset in the page):
[0x0 - 0x3]: 4 bytes, NVDIMM Device Handle, 0 is reserved for NVDIMM
Root device.
+
+ The handle is completely QEMU internal thing, the values in
+ range [0, 0xFFFF] indicate nvdimm device (O means nvdimm
[1, 0xFFFF] indicate nvdimm device
and move 0 to reserved section
Okay.
+ root device named NVDR), other values are reserved by other
s/by/for/
okay.
+ purpose.
s/purpose/purposes/
okay.
+ Current reserved handle:
s/Current reserved handle/Reserved handles/
+ 0x10000 is reserved for QEMU internal DSM function called on
+ the root device.
description is too obscure, I wonder if it could be more specific
I would like to make these reserved values more generic in order to
support more QEMU reserved _DSM methods in the further. So, i planed:
UUID 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62 is dedicated for QEMU reserved
methods. Handle 0x10000 indicates the method is for the root device.
0x10001 ~ 0x1FFFF indicates the method is for the nvdimm devices.
As currently we do not have reserved method on nvdimm device, so i
only document 0x1000 for the root device.
[0x4 - 0x7]: 4 bytes, Revision ID, that is the Arg1 of _DSM method.
[0x8 - 0xB]: 4 bytes. Function Index, that is the Arg2 of _DSM method.
[0xC - 0xFFF]: 4084 bytes, the Arg3 of _DSM method.
@@ -127,6 +137,49 @@ _DSM process diagram:
| result from the page | | |
+--------------------------+ +--------------+
- _FIT implementation
- -------------------
- TODO (will fill it when nvdimm hotplug is introduced)
+QEMU internal use only _DSM function
+------------------------------------
+There is the function introduced by QEMU and only used by QEMU internal.
drop it
+1) Read FIT
+ UUID 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62 is reserved for Read_FIT DSM
+ function (private QEMU function)
not necessary, drop it. Maybe extend UUID description in
"Input parameters:" section
okay.
+ _FIT method uses Read_FIT function to fetch NFIT structures blob from
s/Read_FIT function/_DSM method/
okay.
+ QEMU in 1 page sized increments which are then concatenated and returned
+ as _FIT method result.
+
+ Input parameters:
+ Arg0 – UUID {set to 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62}
+ Arg1 – Revision ID (set to 1)
+ Arg2 - Function Index, 0x1
+ Arg3 - A package containing a buffer whose layout is as follows:
+
+ +----------+--------+--------+-------------------------------------------+
+ | Field | Length | Offset | Description |
+ +----------+--------+--------+-------------------------------------------+
+ | offset | 4 | 0 | offset in QEMU's NFIT structures blob to |
+ | | | | read from |
+ +----------+--------+--------+-------------------------------------------+
+
+ Output:
+ +----------+--------+--------+-------------------------------------------+
+ | Field | Length | Offset | Description |
+ +----------+--------+--------+-------------------------------------------+
+ | | | | return status codes |
+ | | | | 0x100 - error caused by NFIT update while |
+ | status | 4 | 0 | read by _FIT wasn't completed, other |
+ | | | | codes follow Chapter 3 in DSM Spec Rev1 |
+ +----------+--------+--------+-------------------------------------------+
+ | length | 4 | 4 | The fit size |
+ +----------+-----------------+-------------------------------------------+
+ | fit data | Varies | 8 | FIT data, its size is indicated by length |
+ | | | | filed above |
+ +----------+--------+--------+-------------------------------------------+
+
+ The FIT offset is maintained by the OSPM itself, current offset plus
+ the length returned by the function is the next offset we should read.
there shouldn't be 'we' or anything personal in spec
Okay, change it to OSPM. :)
+ When all the FIT data has been read out, zero length is returned.
+
+ If it returns 0x100, OSPM should restart to read FIT (read from offset 0
+ again).
PS:
fix typos and fix spelling/grammatical errors you are adding in above text.
Sorry for the poor English, will check it carefully.
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 9fee077..593ac0d 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -484,6 +484,23 @@ typedef struct NvdimmFuncSetLabelDataIn NvdimmFuncSetLabelDataIn;
QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncSetLabelDataIn) +
offsetof(NvdimmDsmIn, arg3) > 4096);
+struct NvdimmFuncReadFITIn {
+ uint32_t offset; /* the offset into FIT buffer. */
+} QEMU_PACKED;
+typedef struct NvdimmFuncReadFITIn NvdimmFuncReadFITIn;
+QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncReadFITIn) +
+ offsetof(NvdimmDsmIn, arg3) > 4096);
+
+struct NvdimmFuncReadFITOut {
+ /* the size of buffer filled by QEMU. */
+ uint32_t len;
+ uint32_t func_ret_status; /* return status code. */
+ uint32_t length; /* the length of fit. */
redundant as len field above already has it all.
just drop this and describe properly 'len' in spec section
i.e. len: length of entire returned data (including the header)
Okay, i will change the spec like this:
QEMU Writes Output Data (based on the offset in the page):
[0x0 - 0x3]: 4 bytes, length of entire returned data (including the header)
And drop the length field in Read_Fit return buffer, doc
the fit buffer like this:
+----------+--------+--------+-------------------------------------------+
| Field | Length | Offset | Description |
+----------+--------+--------+-------------------------------------------+
| | | | return status codes |
| | | | 0x100 - error caused by NFIT update while |
| status | 4 | 0 | read by _FIT wasn't completed, other |
| | | | codes follow Chapter 3 in DSM Spec Rev1 |
+----------+--------+--------+-------------------------------------------+
| fit data | Varies | 8 | FIT data, The remaining size in the |
| | | | returned buffer is used by FIT |
+----------+--------+--------+-------------------------------------------+
+}
+
+static void nvdimm_dsm_reserved_root(AcpiNVDIMMState *state, NvdimmDsmIn *in,
+ hwaddr dsm_mem_addr)
function name doesn't make any sense to me
As i explained above, handle 0x10000 indicates the reserved _DSM method is
called on the root device...
It makes sense now? :)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
