From: Igor Mammedov <imammedo@redhat.com>
To: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Cc: ehabkost@redhat.com, kvm@vger.kernel.org,
"Michael S. Tsirkin" <mst@redhat.com>,
gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org,
stefanha@redhat.com, pbonzini@redhat.com,
dan.j.williams@intel.com, Laszlo Ersek <lersek@redhat.com>,
rth@twiddle.net
Subject: Re: [Qemu-devel] How to reserve guest physical region for ACPI
Date: Thu, 7 Jan 2016 10:21:57 +0100 [thread overview]
Message-ID: <20160107102157.5fba01cf@nial.brq.redhat.com> (raw)
In-Reply-To: <568BF861.8040907@linux.intel.com>
On Wed, 6 Jan 2016 01:07:45 +0800
Xiao Guangrong <guangrong.xiao@linux.intel.com> wrote:
> On 01/06/2016 12:43 AM, Michael S. Tsirkin wrote:
>
> >>> Yes - if address is static, you need to put it outside
> >>> the table. Can come right before or right after this.
> >>>
> >>>> Also if OperationRegion() is used, then one has to patch
> >>>> DefOpRegion directly as RegionOffset must be Integer,
> >>>> using variable names is not permitted there.
> >>>
> >>> I am not sure the comment was understood correctly.
> >>> The comment says really "we can't use DataTableRegion
> >>> so here is an alternative".
> >> so how are you going to access data at which patched
> >> NameString point to?
> >> for that you'd need a normal patched OperationRegion
> >> as well since DataTableRegion isn't usable.
> >
> > For VMGENID you would patch the method that
> > returns the address - you do not need an op region
> > as you never access it.
> >
> > I don't know about NVDIMM. Maybe OperationRegion can
> > use the patched NameString? Will need some thought.
>
> The ACPI spec says that the offsetTerm in OperationRegion
> is evaluated as Int, so the named object is allowed to be
> used in OperationRegion, that is exact what my patchset
> is doing (http://marc.info/?l=kvm&m=145193395624537&w=2):
that's not my reading of spec:
"
DefOpRegion := OpRegionOp NameString RegionSpace RegionOffset RegionLen
RegionOffset := TermArg => Integer
TermArg := Type2Opcode | DataObject | ArgObj | LocalObj
"
Named object is not allowed per spec, but you've used ArgObj which is
allowed, even Windows ok with such dynamic OperationRegion.
>
> + dsm_mem = aml_arg(3);
> + aml_append(method, aml_store(aml_call0(NVDIMM_GET_DSM_MEM), dsm_mem));
>
> + aml_append(method, aml_operation_region("NRAM", AML_SYSTEM_MEMORY,
> + dsm_mem, TARGET_PAGE_SIZE));
>
> We hide the int64 object which is patched by BIOS in the method,
> NVDIMM_GET_DSM_MEM, to make windows XP happy.
considering that NRAM is allocated in low mem it's even fine to move
OperationRegion into object scope to get rid of IASL warnings
about declariong Named object inside method, but the you'd need to
patch it directly as the only choice for RegionOffset would be DataObject
>
> However, the disadvantages i see are:
> a) as Igor pointed out, we need a way to tell QEMU what is the patched
> address, in NVDIMM ACPI, we used a 64 bit IO ports to pass the address
> to QEMU.
>
> b) BIOS allocated memory is RAM based so it stops us to use MMIO in ACPI,
> MMIO is the more scalable resource than IO port as it has larger region
> and supports 64 bits operation.
next prev parent reply other threads:[~2016-01-07 9:22 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-02 7:20 [Qemu-devel] [PATCH v9 0/5] implement vNVDIMM Xiao Guangrong
2015-12-02 7:20 ` [Qemu-devel] [PATCH v9 1/5] nvdimm: implement NVDIMM device abstract Xiao Guangrong
2015-12-02 7:20 ` [Qemu-devel] [PATCH v9 2/5] acpi: support specified oem table id for build_header Xiao Guangrong
2015-12-02 7:20 ` [Qemu-devel] [PATCH v9 3/5] nvdimm acpi: build ACPI NFIT table Xiao Guangrong
2015-12-02 7:20 ` [Qemu-devel] [PATCH v9 4/5] nvdimm acpi: build ACPI nvdimm devices Xiao Guangrong
2015-12-02 7:21 ` [Qemu-devel] [PATCH v9 5/5] nvdimm: add maintain info Xiao Guangrong
2015-12-10 3:11 ` [Qemu-devel] [PATCH v9 0/5] implement vNVDIMM Xiao Guangrong
2015-12-21 14:13 ` Xiao Guangrong
2015-12-28 2:39 ` [Qemu-devel] How to reserve guest physical region for ACPI Xiao Guangrong
2015-12-28 12:50 ` Michael S. Tsirkin
2015-12-30 15:55 ` Igor Mammedov
2015-12-30 19:52 ` Michael S. Tsirkin
2016-01-04 20:17 ` Laszlo Ersek
2016-01-05 17:08 ` Igor Mammedov
2016-01-05 17:22 ` Laszlo Ersek
2016-01-06 13:39 ` Igor Mammedov
2016-01-06 14:43 ` Laszlo Ersek
2016-01-07 13:51 ` Igor Mammedov
2016-01-07 17:33 ` Laszlo Ersek
2016-01-05 16:30 ` Igor Mammedov
2016-01-05 16:43 ` Michael S. Tsirkin
2016-01-05 17:07 ` Laszlo Ersek
2016-01-05 17:07 ` Xiao Guangrong
2016-01-07 9:21 ` Igor Mammedov [this message]
2016-01-08 4:21 ` Xiao Guangrong
2016-01-08 9:42 ` Laszlo Ersek
2016-01-08 15:59 ` Igor Mammedov
2016-01-07 10:30 ` Igor Mammedov
2016-01-07 10:54 ` Michael S. Tsirkin
2016-01-07 13:42 ` Igor Mammedov
2016-01-07 17:11 ` Laszlo Ersek
2016-01-07 17:08 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160107102157.5fba01cf@nial.brq.redhat.com \
--to=imammedo@redhat.com \
--cc=dan.j.williams@intel.com \
--cc=ehabkost@redhat.com \
--cc=gleb@kernel.org \
--cc=guangrong.xiao@linux.intel.com \
--cc=kvm@vger.kernel.org \
--cc=lersek@redhat.com \
--cc=mst@redhat.com \
--cc=mtosatti@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).