Re: [Qemu-devel] [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical address space

[Jan Kiszka]

On 2012-11-05 03:42, Hao, Xudong wrote:
>> -----Original Message-----
>> From: Jan Kiszka [mailto:address@hidden
>> Sent: Sunday, November 04, 2012 8:55 PM
>> To: Hao, Xudong
>> Cc: address@hidden; address@hidden; address@hidden
>> Subject: Re: [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical
>> address space
>>
>> On 2012-11-04 13:15, Hao, Xudong wrote:
>>>> -----Original Message-----
>>>> From: Jan Kiszka [mailto:address@hidden
>>>> Sent: Saturday, November 03, 2012 6:55 PM
>>>> To: Hao, Xudong
>>>> Cc: address@hidden; address@hidden; address@hidden
>>>> Subject: Re: [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical
>>>> address space
>>>>
>>>> On 2012-11-02 06:38, Xudong Hao wrote:
>>>>> For 64 bit processor, emulate 40 bits physical address if the host 
>>>>> physical
>>>>> address space >= 40bits, else guest physical is same as host.
>>>>>
>>>>> Signed-off-by: Xudong Hao <address@hidden>
>>>>> ---
>>>>>  target-i386/cpu.c |    5 ++++-
>>>>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>>>>> index 423e009..3a78881 100644
>>>>> --- a/target-i386/cpu.c
>>>>> +++ b/target-i386/cpu.c
>>>>> @@ -1584,7 +1584,10 @@ void cpu_x86_cpuid(CPUX86State *env,
>> uint32_t
>>>> index, uint32_t count,
>>>>>          if (env->cpuid_ext2_features & CPUID_EXT2_LM) {
>>>>>              /* 64 bit processor */
>>>>>  /* XXX: The physical address space is limited to 42 bits in exec.c. */
>>>>> -            *eax = 0x00003028;   /* 48 bits virtual, 40 bits physical
>> */
>>>>> +/* XXX: 40 bits physical if host physical address space >= 40 bits */
>>>>> +            uint32_t a, b, c, d;
>>>>> +            host_cpuid(0x80000008, 0, &a, &b, &c, &d);
>>>>> +            *eax = a < 0x00003028 ? a : 0x00003028;
>>>>
>>>> This variation will not only affect -cpu host, right? That can create
>>>> problems when migrating between hosts with different address widths, and
>>>> then we will need some control knob to adjust what it reported to the 
>>>> guest.
>>>>
>>>
>>> Oh, I did not consider migrating to different platform(addr widths).
>>> But I think the fixed value 40 bits may cause problem: in VT-d case, when a
>> host support GAW < 40 bits, and qemu emulate 40 bits guest physical address
>> space, will bring bug on:
>>>
>>> drivers/iommu/intel-iommu.c
>>> static struct dma_pte *pfn_to_dma_pte(struct dmar_domain *domain,
>>>                       unsigned long pfn, int target_level)
>>> {
>>>     int addr_width = agaw_to_width(domain->agaw) - VTD_PAGE_SHIFT;
>>> ...
>>>     BUG_ON(!domain->pgd);
>>>     BUG_ON(addr_width < BITS_PER_LONG && pfn >> addr_width);
>>>
>>
>> Does it mean that buggy or malicious user space can trigger a kernel
>> bug? Then this must be fixed of course.
>>
> Probably yes, when guest RAM is large enough or allocate MMIO to very high 
> address.

...and those things are under user space control. If you have an idea
how to trigger this, please give it a try. This is an availability issue
as untrusted user space could bring down the whole system.

> 
> Jan, I'm not familiar the migration, do you have interest to add the 
> migration part fixing?
> 

I'm not up to date with what is going on in the context of CPU feature
configuration, CC'ing folks who reworked this recently.

In any case, the general pattern is: make this configurable (=> CPU
feature flag) and then possibly also adjust it for compat QEMU machine
types.

Jan

signature.asc
Description: OpenPGP digital signature