Re: [Xen-devel] [PATCH] x86/mem_sharing: Rectify test for "empty" physmap entry in sharing_add_to_physmap

["Andres Lagar-Cavilla" <andres@xxxxxxxxxxxxxxxx>]

> At 07:18 -0700 on 18 Apr (1334733529), Andres Lagar-Cavilla wrote:
>> > At 09:06 -0400 on 18 Apr (1334740000), Andres Lagar-Cavilla wrote:
>> >>  xen/arch/x86/mm/mem_sharing.c |  6 ++++--
>> >>  1 files changed, 4 insertions(+), 2 deletions(-)
>> >>
>> >>
>> >> Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
>> >>
>> >> diff -r 8609bbbba141 -r 495d3df95c1d xen/arch/x86/mm/mem_sharing.c
>> >> --- a/xen/arch/x86/mm/mem_sharing.c
>> >> +++ b/xen/arch/x86/mm/mem_sharing.c
>> >> @@ -1073,9 +1073,11 @@ int mem_sharing_add_to_physmap(struct do
>> >>      if ( spage->sharing->handle != sh )
>> >>          goto err_unlock;
>> >>
>> >> -    /* Make sure the target page is a hole in the physmap */
>> >> +    /* Make sure the target page is a hole in the physmap. This is
>> not
>> >> as
>> >> +     * simple a test as we'd like it to be. Non-existent p2m entries
>> >> return
>> >> +     * invalid mfn and p2m_mmio_dm type when queried. */
>> >>      if ( mfn_valid(cmfn) ||
>> >> -         (!(p2m_is_ram(cmfn_type))) )
>> >> +         ( (!(p2m_is_ram(cmfn_type))) && (cmfn_type != p2m_mmio_dm)
>> ) )
>> >
>> > This test looks wrong, even after the fix.  I think it should be
>> testing
>> > for cmfn_type == p2m_mmio_dm || cmfn_type == p2m_invalid and ignoring
>> > mfn_valid().
>>
>> Maybe :)
>>
>> I'm coming up with the semantics for this one, loosely based on  the
>> regular populate physmap code path. That one can end up replacing
>> existing
>> regular pages, or paged out entries, or PoD entries, with the new
>> populated pages (in guest_physmap_add_entry). I don't know that all of
>> the
>> above is reasonable.
>>
>> But certainly I would like to keep the ability to replace paged out
>> entries with (identical) pages that are already present in other
>> domains.
>
> Fair enough.  Maybe you could define a new p2m-type-mask of all the
> things you think it's OK to replace in this kind of situation, and apply
> it in all cases?

Is there a chance for a p2m_mmio_dm entry to hold a valid mfn?
Thanks,
Andres

>
> Tim.
>



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel