[PATCH v2.0 0/6] Add memory add support to Xen

[PATCH v2.0 0/6] Add memory add support to Xen

[Jiang, Yunhong]

This patchset is an update for previous one to support memory add.

The change includes 
a) changes to boot allocator, frametable setup, m2p table setup, so that they will be called not-only when system boot up, but also when hot add happens
b) Changes to page fault handler, to propgate frametable/m2p table changes to all guest.

Comparing to previous patchset, the difference is:
a) Rename the function name of boot allacator extension.
b) Fix compatibility mode bugs. One is, we will not update the m2p_compat_vstart if we support memory add. 

Thanks
Yunhong Jiang

Re: [PATCH v2.0 0/6] Add memory add support to Xen

[Jan Beulich]

>>> "Jiang, Yunhong" <yunhong.jiang@intel.com> 08.07.09 09:43 >>>
>This patchset is an update for previous one to support memory add.
>
>The change includes 
>a) changes to boot allocator, frametable setup, m2p table setup, so that they will be called not-only when system boot up, but also
>when hot add happens
>b) Changes to page fault handler, to propgate frametable/m2p table changes to all guest.
>
>Comparing to previous patchset, the difference is:
>a) Rename the function name of boot allacator extension.
>b) Fix compatibility mode bugs. One is, we will not update the m2p_compat_vstart if we support memory add. 

There's one other problem with this overall change: Non-pv-ops pv Linux guests (all versions afaict) establish an upper bound on the m2p table size during early boot, and use this to bound check MFNs before accessing the array (see the setup and use of machine_to_phys_order). Hence, when you grow the m2p table, you might need to send some sort of notification to all pv domains so that they can adjust that upper bound. If not a notification, some other communication mechanism will be needed (i.e. a new ELF note). Hot-added memory must never be made visible to a pv guest not supporting this new protocol (in particular, hot add may need to be disabled altogether if Dom0 doesn't support it).

As to pv-ops currently not being affected by this - the respective check currently sits in an #if 0 conditional, but certainly this is a latent bug (becoming a real one as soon as Dom0 or device pass-through come into the picture): Since without the check unbounded MFNs can be used to index into the array, it is possible to access I/O memory here, so simply being prepared to handle a fault resulting from an out-of-bounds access isn't enough. The minimally required boundary check is to make sure the resulting address is still inside hypervisor space (under the assumption that the hypervisor will itself never make I/O memory addressable for the guest).

Jan

Re: [PATCH v2.0 0/6] Add memory add support to Xen

[Keir Fraser]

On 10/07/2009 08:16, "Jan Beulich" <JBeulich@novell.com> wrote:

> There's one other problem with this overall change: Non-pv-ops pv Linux guests
> (all versions afaict) establish an upper bound on the m2p table size during
> early boot, and use this to bound check MFNs before accessing the array (see
> the setup and use of machine_to_phys_order). Hence, when you grow the m2p
> table, you might need to send some sort of notification to all pv domains so
> that they can adjust that upper bound. If not a notification, some other
> communication mechanism will be needed (i.e. a new ELF note). Hot-added memory
> must never be made visible to a pv guest not supporting this new protocol (in
> particular, hot add may need to be disabled altogether if Dom0 doesn't support
> it).

The correct answer I think is for Xen to specify a machine_to_phys order
that corresponds to the size of the M2P 'hole' rather than to the actual
amount of memory currently populated on this host. The extra inefficiency is
only that some I/O MFNs may be detected via fault rather than out-of-bounds
check (and then probably only on systems with <4G RAM).

This for x86/64 guests of course. We already established that compat guests
and memory add are going to have lesser mutual support.

 -- Keir

> As to pv-ops currently not being affected by this - the respective check
> currently sits in an #if 0 conditional, but certainly this is a latent bug
> (becoming a real one as soon as Dom0 or device pass-through come into the
> picture): Since without the check unbounded MFNs can be used to index into the
> array, it is possible to access I/O memory here, so simply being prepared to
> handle a fault resulting from an out-of-bounds access isn't enough. The
> minimally required boundary check is to make sure the resulting address is
> still inside hypervisor space (under the assumption that the hypervisor will
> itself never make I/O memory addressable for the guest).

RE: [PATCH v2.0 0/6] Add memory add support to Xen

[Jiang, Yunhong]

Keir Fraser wrote:
> On 10/07/2009 08:16, "Jan Beulich" <JBeulich@novell.com> wrote:
> 
>> There's one other problem with this overall change: Non-pv-ops pv
>> Linux guests (all versions afaict) establish an upper bound on the
>> m2p table size during early boot, and use this to bound check MFNs
>> before accessing the array (see the setup and use of
>> machine_to_phys_order). Hence, when you grow the m2p table, you
>> might need to send some sort of notification to all pv domains so
>> that they can adjust that upper bound. If not a notification, some
>> other communication mechanism will be needed (i.e. a new ELF note).
>> Hot-added memory must never be made visible to a pv guest not
>> supporting this new protocol (in particular, hot add may need to be
>> disabled altogether if Dom0 doesn't support it). 
> 
> The correct answer I think is for Xen to specify a
> machine_to_phys order
> that corresponds to the size of the M2P 'hole' rather than to the
> actual amount of memory currently populated on this host. The extra
> inefficiency is only that some I/O MFNs may be detected via fault
> rather than out-of-bounds check (and then probably only on systems
> with <4G RAM). 
> 
> This for x86/64 guests of course. We already established that compat
> guests and memory add are going to have lesser mutual support.
> 
> -- Keir

 I checked this before and I thought it is ok.
Currently the machine_to_phys_order is caculated based on return value of XENMEM_machphys_mapping. For both x86_32 and non-compat x86_64, this size will not be adjusted dynamically, so it is ok (it will cover the whole possible range).
The only issue is for compatible domain. For compatible domain, the value returned in XENMEM_machphys_mapping is adjusted (i.e. MACH2PHYS_COMPAT_VIRT_START(d)). However, domain_clamp_alloc_bitsize() in domainheap allocator will make sure the hot-added memory will not be assigned to the guest.

Did I miss-understand something?

Thanks
Yunhong Jiang

> 
>> As to pv-ops currently not being affected by this - the respective
>> check currently sits in an #if 0 conditional, but certainly this is
>> a latent bug (becoming a real one as soon as Dom0 or device
>> pass-through come into the picture): Since without the check
>> unbounded MFNs can be used to index into the array, it is possible
>> to access I/O memory here, so simply being prepared to handle a
>> fault resulting from an out-of-bounds access isn't enough. The
>> minimally required boundary check is to make sure the resulting
>> address is still inside hypervisor space (under the assumption that
>> the hypervisor will itself never make I/O memory addressable for the
>> guest).  

RE: [PATCH v2.0 0/6] Add memory add support to Xen

[Jiang, Yunhong]

Keir Fraser wrote:
> On 10/07/2009 08:16, "Jan Beulich" <JBeulich@novell.com> wrote:
> 
>> There's one other problem with this overall change: Non-pv-ops pv
>> Linux guests (all versions afaict) establish an upper bound on the
>> m2p table size during early boot, and use this to bound check MFNs
>> before accessing the array (see the setup and use of
>> machine_to_phys_order). Hence, when you grow the m2p table, you
>> might need to send some sort of notification to all pv domains so
>> that they can adjust that upper bound. If not a notification, some
>> other communication mechanism will be needed (i.e. a new ELF note).
>> Hot-added memory must never be made visible to a pv guest not
>> supporting this new protocol (in particular, hot add may need to be
>> disabled altogether if Dom0 doesn't support it). 
> 
> The correct answer I think is for Xen to specify a
> machine_to_phys order
> that corresponds to the size of the M2P 'hole' rather than to the
> actual amount of memory currently populated on this host. The extra

>From the hypercall of XENMEM_machphys_mapping, seems it return the m2p hole already (I mean non-compat guest), so why it is "actual amount of memory"?

    case XENMEM_machphys_mapping:
    {
        static const struct xen_machphys_mapping mapping = {
            .v_start = MACH2PHYS_VIRT_START,
            .v_end   = MACH2PHYS_VIRT_END,
            .max_mfn = MACH2PHYS_NR_ENTRIES - 1
        };

        if ( copy_to_guest(arg, &mapping, 1) )
            return -EFAULT;

        return 0;
    }

Thanks
Yunhong Jiang


> inefficiency is only that some I/O MFNs may be detected via fault
> rather than out-of-bounds check (and then probably only on systems
> with <4G RAM). 
> 
> This for x86/64 guests of course. We already established that compat
> guests and memory add are going to have lesser mutual support.
> 
> -- Keir
> 
>> As to pv-ops currently not being affected by this - the respective
>> check currently sits in an #if 0 conditional, but certainly this is
>> a latent bug (becoming a real one as soon as Dom0 or device
>> pass-through come into the picture): Since without the check
>> unbounded MFNs can be used to index into the array, it is possible
>> to access I/O memory here, so simply being prepared to handle a
>> fault resulting from an out-of-bounds access isn't enough. The
>> minimally required boundary check is to make sure the resulting
>> address is still inside hypervisor space (under the assumption that
>> the hypervisor will itself never make I/O memory addressable for the
>> guest).  

Re: [PATCH v2.0 0/6] Add memory add support to Xen

[Keir Fraser]

On 10/07/2009 09:32, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote:

>> This for x86/64 guests of course. We already established that compat
>> guests and memory add are going to have lesser mutual support.
> 
>  I checked this before and I thought it is ok.
> Currently the machine_to_phys_order is caculated based on return value of
> XENMEM_machphys_mapping. For both x86_32 and non-compat x86_64, this size will
> not be adjusted dynamically, so it is ok (it will cover the whole possible
> range).
> The only issue is for compatible domain. For compatible domain, the value
> returned in XENMEM_machphys_mapping is adjusted (i.e.
> MACH2PHYS_COMPAT_VIRT_START(d)). However, domain_clamp_alloc_bitsize() in
> domainheap allocator will make sure the hot-added memory will not be assigned
> to the guest.
> 
> Did I miss-understand something?

Sounds okay to me. Perhaps Jan has other thoughts?

 -- Keir

Re: [PATCH v2.0 0/6] Add memory add support to Xen

[Jan Beulich]

>>> Keir Fraser <keir.fraser@eu.citrix.com> 10.07.09 10:38 >>>
>On 10/07/2009 09:32, "Jiang, Yunhong" <yunhong.jiang@intel.com> wrote:
>
>>> This for x86/64 guests of course. We already established that compat
>>> guests and memory add are going to have lesser mutual support.
>> 
>>  I checked this before and I thought it is ok.
>> Currently the machine_to_phys_order is caculated based on return value of
>> XENMEM_machphys_mapping. For both x86_32 and non-compat x86_64, this size will
>> not be adjusted dynamically, so it is ok (it will cover the whole possible
>> range).
>> The only issue is for compatible domain. For compatible domain, the value
>> returned in XENMEM_machphys_mapping is adjusted (i.e.
>> MACH2PHYS_COMPAT_VIRT_START(d)). However, domain_clamp_alloc_bitsize() in
>> domainheap allocator will make sure the hot-added memory will not be assigned
>> to the guest.
>> 
>> Did I miss-understand something?
>
>Sounds okay to me. Perhaps Jan has other thoughts?

Oh, indeed - somehow I (incorrectly) recalled that this hypercall returned the
actually used boundary rather than the highest possible one. With me being
wrong here, all should be fine with that change.

Sorry for the noise,
Jan

RE: Re: [PATCH v2.0 0/6] Add memory add support to Xen

[Jiang, Yunhong]

xen-devel-bounces@lists.xensource.com wrote:
>>>> Keir Fraser <keir.fraser@eu.citrix.com> 10.07.09 10:38 >>>
>> On 10/07/2009 09:32, "Jiang, Yunhong" <yunhong.jiang@intel.com>
>> wrote: 
>> 
>>>> This for x86/64 guests of course. We already established that
>>>> compat guests and memory add are going to have lesser mutual
>>>> support. 
>>> 
>>>  I checked this before and I thought it is ok.
>>> Currently the machine_to_phys_order is caculated based on return
>>> value of XENMEM_machphys_mapping. For both x86_32 and non-compat
>>> x86_64, this size will not be adjusted dynamically, so it is ok (it
>>> will cover the whole possible range). The only issue is for
>>> compatible domain. For compatible domain, the value returned in
>>> XENMEM_machphys_mapping is adjusted (i.e.
>>> MACH2PHYS_COMPAT_VIRT_START(d)). However, 
> domain_clamp_alloc_bitsize() in
>>> domainheap allocator will make sure the hot-added memory will not
>>> be assigned to the guest. 
>>> 
>>> Did I miss-understand something?
>> 
>> Sounds okay to me. Perhaps Jan has other thoughts?
> 
> Oh, indeed - somehow I (incorrectly) recalled that this hypercall
> returned the actually used boundary rather than the highest possible
> one. With me being wrong here, all should be fine with that change.
> 
> Sorry for the noise,
> Jan

Thanks for your review and consideration indeed !

--jyh

> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel