[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest ph
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] qemu-kvm/cpuid: fix a emulation of guest physical address space |
Date: |
Thu, 8 Nov 2012 14:40:20 -0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, Nov 05, 2012 at 07:22:55AM +0100, Jan Kiszka wrote:
> 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.
We can't automatically expose data derived from host capabilities to the
guest automatically, as this breaks live migration. This is probably
better handled by adding a new property to the X86CPU class.
If you really want to, you can add a "host" or "auto" mode, too, for
users that don't care about live migration. But that mode can't be
enabled by default (but it could be enabled by -cpu host).
--
Eduardo