Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Xen: Devel

[PATCH 2 of 2] x86/mem_sharing: Rectify test for "empty" physmap entry in sharing_add_to_physmap

 

 

Xen devel RSS feed   Index | Next | Previous | View Threaded


andres at lagarcavilla

Apr 24, 2012, 12:20 PM

Post #1 of 3 (87 views)
Permalink
[PATCH 2 of 2] x86/mem_sharing: Rectify test for "empty" physmap entry in sharing_add_to_physmap

xen/arch/x86/mm/mem_sharing.c | 7 ++++---
xen/include/asm-x86/p2m.h | 8 ++++++++
2 files changed, 12 insertions(+), 3 deletions(-)


Signed-off-by: Andres Lagar-Cavilla <andres [at] lagarcavilla>

diff -r 5be9a05f17fd -r 51646b89b182 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,10 @@ 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 */
- if ( mfn_valid(cmfn) ||
- (!(p2m_is_ram(cmfn_type))) )
+ /* Make sure the target page is a hole in the physmap. These are typically
+ * p2m_mmio_dm, but also accept p2m_invalid and paged out pages. See the
+ * definition of p2m_is_hole in p2m.h. */
+ if ( !p2m_is_hole(cmfn_type) || mfn_valid(cmfn) )
{
ret = XENMEM_SHARING_OP_C_HANDLE_INVALID;
goto err_unlock;
diff -r 5be9a05f17fd -r 51646b89b182 xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -133,6 +133,13 @@ typedef unsigned int p2m_query_t;
| p2m_to_mask(p2m_ram_paging_in) \
| p2m_to_mask(p2m_ram_shared))

+/* Types that represent a physmap hole that is ok to replace with a shared
+ * entry */
+#define P2M_HOLE_TYPES (p2m_to_mask(p2m_mmio_dm) \
+ | p2m_to_mask(p2m_invalid) \
+ | p2m_to_mask(p2m_ram_paging_in) \
+ | p2m_to_mask(p2m_ram_paged))
+
/* Grant mapping types, which map to a real machine frame in another
* VM */
#define P2M_GRANT_TYPES (p2m_to_mask(p2m_grant_map_rw) \
@@ -173,6 +180,7 @@ typedef unsigned int p2m_query_t;

/* Useful predicates */
#define p2m_is_ram(_t) (p2m_to_mask(_t) & P2M_RAM_TYPES)
+#define p2m_is_hole(_t) (p2m_to_mask(_t) & P2M_HOLE_TYPES)
#define p2m_is_mmio(_t) (p2m_to_mask(_t) & P2M_MMIO_TYPES)
#define p2m_is_readonly(_t) (p2m_to_mask(_t) & P2M_RO_TYPES)
#define p2m_is_magic(_t) (p2m_to_mask(_t) & P2M_MAGIC_TYPES)

_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


tim at xen

Apr 25, 2012, 5:41 AM

Post #2 of 3 (79 views)
Permalink
Re: [PATCH 2 of 2] x86/mem_sharing: Rectify test for "empty" physmap entry in sharing_add_to_physmap [In reply to]

At 15:20 -0400 on 24 Apr (1335280819), Andres Lagar-Cavilla wrote:
> xen/arch/x86/mm/mem_sharing.c | 7 ++++---
> xen/include/asm-x86/p2m.h | 8 ++++++++
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
>
> Signed-off-by: Andres Lagar-Cavilla <andres [at] lagarcavilla>
>
> diff -r 5be9a05f17fd -r 51646b89b182 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,10 @@ 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 */
> - if ( mfn_valid(cmfn) ||
> - (!(p2m_is_ram(cmfn_type))) )
> + /* Make sure the target page is a hole in the physmap. These are typically
> + * p2m_mmio_dm, but also accept p2m_invalid and paged out pages. See the
> + * definition of p2m_is_hole in p2m.h. */
> + if ( !p2m_is_hole(cmfn_type) || mfn_valid(cmfn) )

Hmm. Is the mfn_valid() to handle p2m_ram_paging_in entries sometimes
having an MFN and sometimes not? I think it would be nicer to either
always replace paging-in pages or never do it. In any case, it's bogus
to test mfn_valid() for any of the other types.

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel


andres at lagarcavilla

Apr 25, 2012, 8:20 AM

Post #3 of 3 (82 views)
Permalink
Re: [PATCH 2 of 2] x86/mem_sharing: Rectify test for "empty" physmap entry in sharing_add_to_physmap [In reply to]

> At 15:20 -0400 on 24 Apr (1335280819), Andres Lagar-Cavilla wrote:
>> xen/arch/x86/mm/mem_sharing.c | 7 ++++---
>> xen/include/asm-x86/p2m.h | 8 ++++++++
>> 2 files changed, 12 insertions(+), 3 deletions(-)
>>
>>
>> Signed-off-by: Andres Lagar-Cavilla <andres [at] lagarcavilla>
>>
>> diff -r 5be9a05f17fd -r 51646b89b182 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,10 @@ 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 */
>> - if ( mfn_valid(cmfn) ||
>> - (!(p2m_is_ram(cmfn_type))) )
>> + /* Make sure the target page is a hole in the physmap. These are
>> typically
>> + * p2m_mmio_dm, but also accept p2m_invalid and paged out pages.
>> See the
>> + * definition of p2m_is_hole in p2m.h. */
>> + if ( !p2m_is_hole(cmfn_type) || mfn_valid(cmfn) )
>
> Hmm. Is the mfn_valid() to handle p2m_ram_paging_in entries sometimes
> having an MFN and sometimes not? I think it would be nicer to either
> always replace paging-in pages or never do it. In any case, it's bogus
> to test mfn_valid() for any of the other types.

Yes, that is the concern. paging_in entries may have a backing mfn. I am
not opposed to just replacing the entry if it is in paging_in state,
regardless of the mfn. It would be similar to any of the cases in which
guest_physmap_add_entry is used with a valid mfn in the entry to be
updated.

Andres

>
> Cheers,
>
> Tim.
>



_______________________________________________
Xen-devel mailing list
Xen-devel [at] lists
http://lists.xen.org/xen-devel

Xen devel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.