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

[Tim Deegan <tim@xxxxxxx>]

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

Cheers,

Tim.

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