Xen devel - Jul 2009 - [PATCH v2.0 0/6] Add memory add support to Xen

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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>>> "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


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

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).


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

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).  
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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).  
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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



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

>>> 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




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

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
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel