Re: [Xen-devel] [PATCH v4 10/16] xen/mm: Switch map_pages_to_xen to use MFN typesafe

2018-03-05 Thread Julien Grall 

Hi,

On 05/03/18 14:39, Jan Beulich wrote:

On 05.03.18 at 15:07,  wrote:

On 02/03/18 15:06, Jan Beulich wrote:

On 21.02.18 at 15:02,  wrote:

--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -40,6 +40,10 @@ asm(".file \"" __FILE__ "\"");
   #include 
   #include 
   
+/* Override macros from asm/page.h to make them work with mfn_t */

+#undef page_to_mfn
+#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))


I can't spot where this is needed in this file.


@@ -234,7 +238,7 @@ void vunmap(const void *va)
   #ifndef _PAGE_NONE
   destroy_xen_mappings(addr, addr + PAGE_SIZE * pages);
   #else /* Avoid tearing down intermediate page tables. */
-map_pages_to_xen(addr, 0, pages, _PAGE_NONE);
+map_pages_to_xen(addr, _mfn(0), pages, _PAGE_NONE);


INVALID_MFN?


I can but then we end up to the same clumsiness as you mention in #8. So
what's your preference?


My preference is to skip the increments for INVALID_MFN in patch 8.
Since you don't want to do that, I guess we'll have to live with zero
being used everywhere.


It is not that I don't want. I don't have any x86 setup and not that 
confident to provide a change bigger than switch from plain unsigned int 
to typesafe and light clean-up.


I can provide a patch if you really want. But likely it is going to be 
quicker for you to write it.


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 10/16] xen/mm: Switch map_pages_to_xen to use MFN typesafe

2018-03-05 Thread Jan Beulich 
>>> On 05.03.18 at 15:07,  wrote:
> On 02/03/18 15:06, Jan Beulich wrote:
> On 21.02.18 at 15:02,  wrote:
>>> --- a/xen/arch/x86/x86_64/mm.c
>>> +++ b/xen/arch/x86/x86_64/mm.c
>>> @@ -40,6 +40,10 @@ asm(".file \"" __FILE__ "\"");
>>>   #include 
>>>   #include 
>>>   
>>> +/* Override macros from asm/page.h to make them work with mfn_t */
>>> +#undef page_to_mfn
>>> +#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
>> 
>> I can't spot where this is needed in this file.
>> 
>>> @@ -234,7 +238,7 @@ void vunmap(const void *va)
>>>   #ifndef _PAGE_NONE
>>>   destroy_xen_mappings(addr, addr + PAGE_SIZE * pages);
>>>   #else /* Avoid tearing down intermediate page tables. */
>>> -map_pages_to_xen(addr, 0, pages, _PAGE_NONE);
>>> +map_pages_to_xen(addr, _mfn(0), pages, _PAGE_NONE);
>> 
>> INVALID_MFN?
> 
> I can but then we end up to the same clumsiness as you mention in #8. So 
> what's your preference?

My preference is to skip the increments for INVALID_MFN in patch 8.
Since you don't want to do that, I guess we'll have to live with zero
being used everywhere.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 10/16] xen/mm: Switch map_pages_to_xen to use MFN typesafe

2018-03-05 Thread Julien Grall 

Hi,

On 02/03/18 15:06, Jan Beulich wrote:

On 21.02.18 at 15:02,  wrote:

--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -40,6 +40,10 @@ asm(".file \"" __FILE__ "\"");
  #include 
  #include 
  
+/* Override macros from asm/page.h to make them work with mfn_t */

+#undef page_to_mfn
+#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))


I can't spot where this is needed in this file.


@@ -234,7 +238,7 @@ void vunmap(const void *va)
  #ifndef _PAGE_NONE
  destroy_xen_mappings(addr, addr + PAGE_SIZE * pages);
  #else /* Avoid tearing down intermediate page tables. */
-map_pages_to_xen(addr, 0, pages, _PAGE_NONE);
+map_pages_to_xen(addr, _mfn(0), pages, _PAGE_NONE);


INVALID_MFN?


I can but then we end up to the same clumsiness as you mention in #8. So 
what's your preference?




Also please again see about using PFN_DOWN() for some of the
sizes passed to the function as you touch those places anyway.


Will do.



Jan



--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 10/16] xen/mm: Switch map_pages_to_xen to use MFN typesafe

2018-03-02 Thread Jan Beulich 
>>> On 02.03.18 at 16:06,  wrote:
 On 21.02.18 at 15:02,  wrote:
>> --- a/xen/arch/x86/x86_64/mm.c
>> +++ b/xen/arch/x86/x86_64/mm.c
>> @@ -40,6 +40,10 @@ asm(".file \"" __FILE__ "\"");
>>  #include 
>>  #include 
>>  
>> +/* Override macros from asm/page.h to make them work with mfn_t */
>> +#undef page_to_mfn
>> +#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
> 
> I can't spot where this is needed in this file.

Oh, I'm sorry, I should have looked at the file as well, not just
at the hunks in the patch.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 10/16] xen/mm: Switch map_pages_to_xen to use MFN typesafe

2018-03-02 Thread Jan Beulich 
>>> On 21.02.18 at 15:02,  wrote:
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -40,6 +40,10 @@ asm(".file \"" __FILE__ "\"");
>  #include 
>  #include 
>  
> +/* Override macros from asm/page.h to make them work with mfn_t */
> +#undef page_to_mfn
> +#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))

I can't spot where this is needed in this file.

> @@ -234,7 +238,7 @@ void vunmap(const void *va)
>  #ifndef _PAGE_NONE
>  destroy_xen_mappings(addr, addr + PAGE_SIZE * pages);
>  #else /* Avoid tearing down intermediate page tables. */
> -map_pages_to_xen(addr, 0, pages, _PAGE_NONE);
> +map_pages_to_xen(addr, _mfn(0), pages, _PAGE_NONE);

INVALID_MFN?

Also please again see about using PFN_DOWN() for some of the
sizes passed to the function as you touch those places anyway.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 10/16] xen/mm: Switch map_pages_to_xen to use MFN typesafe

2018-02-23 Thread Wei Liu 
On Wed, Feb 21, 2018 at 02:02:53PM +, Julien Grall wrote:
> The current prototype is slightly confusing because it takes a virtual
> address and a physical frame (not address!). Switching to MFN will improve
> safety and reduce the chance to mistakenly invert the 2 parameters.
> 
> Signed-off-by: Julien Grall 

Reviewed-by: Wei Liu 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v4 10/16] xen/mm: Switch map_pages_to_xen to use MFN typesafe

2018-02-22 Thread Tian, Kevin 
> From: Julien Grall [mailto:julien.gr...@arm.com]
> Sent: Wednesday, February 21, 2018 10:03 PM
> 
> The current prototype is slightly confusing because it takes a virtual
> address and a physical frame (not address!). Switching to MFN will improve
> safety and reduce the chance to mistakenly invert the 2 parameters.
> 
> Signed-off-by: Julien Grall 

Reviewed-by: Kevin Tian 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel