Re: [Qemu-devel] How to reserve guest physical region for ACPI

[Laszlo Ersek]

On 01/08/16 05:21, Xiao Guangrong wrote:
> 
> 
> On 01/07/2016 05:21 PM, Igor Mammedov wrote:
>> On Wed, 6 Jan 2016 01:07:45 +0800
>> Xiao Guangrong <address@hidden> 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.
> 
> Sorry, Named object i was talking about is something like this:
> Name("SOTH", int(0x10000))
> 
> I am checking acpi spec, and this is a formal NamedObj definition in
> that spec, my fault.
> 
>>
>>>
>>> +    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
>>
> 
> Yes, it is. So it is depends on the question in my reply of another thread:
> http://marc.info/?l=kvm&m=145222487605390&w=2
> Can we assume that BIOS allocated address is always 32 bits?

As far as OVMF is concerned: you can assume this at the moment, yes.

Thanks
Laszlo

> If yes, we also need not make ssdt as v2.