Re: [Xen-devel] [PATCH] xen: support 52 bit physical addresses in pv guests

2017-09-21 Thread Boris Ostrovsky 



On 09/21/2017 12:16 PM, Andrew Cooper wrote:

On 21/09/17 17:00, Boris Ostrovsky wrote:








Signed-off-by: Juergen Gross 
---
   arch/x86/include/asm/xen/page.h | 11 ++-
   arch/x86/xen/mmu_pv.c   |  4 ++--
   2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/xen/page.h
b/arch/x86/include/asm/xen/page.h
index 07b6531813c4..bcb8b193c8d1 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -26,6 +26,15 @@ typedef struct xpaddr {
   phys_addr_t paddr;
   } xpaddr_t;
   +#ifdef CONFIG_X86_64
+#define XEN_PHYSICAL_MASK    ((1UL << 52) - 1)



SME is not supported for PV guests but for consistency (and in case sme
bit somehow gets set)
#define XEN_PHYSICAL_MASK    __sme_clr(((1UL << 52) - 1))


Hmm, really? Shouldn't we rather add something like

BUG_ON(sme_active());

somewhere?


We can do that too.


Please don't do anything to cause Linux to crash if Xen is using SME 
itself, but leaving all of the PV guest unencrypted.


sme_active() returns true if the *guest* enables it.

Also, if the guest's memory is unencrypted, doesn't this mean that mfns 
that it sees (or, rather, ptes) will not have the SME bit set?


-boris

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

Re: [Xen-devel] [PATCH] xen: support 52 bit physical addresses in pv guests

2017-09-21 Thread Andrew Cooper 

On 21/09/17 17:00, Boris Ostrovsky wrote:








Signed-off-by: Juergen Gross 
---
   arch/x86/include/asm/xen/page.h | 11 ++-
   arch/x86/xen/mmu_pv.c   |  4 ++--
   2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/xen/page.h
b/arch/x86/include/asm/xen/page.h
index 07b6531813c4..bcb8b193c8d1 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -26,6 +26,15 @@ typedef struct xpaddr {
   phys_addr_t paddr;
   } xpaddr_t;
   +#ifdef CONFIG_X86_64
+#define XEN_PHYSICAL_MASK    ((1UL << 52) - 1)



SME is not supported for PV guests but for consistency (and in case sme
bit somehow gets set)
#define XEN_PHYSICAL_MASK    __sme_clr(((1UL << 52) - 1))


Hmm, really? Shouldn't we rather add something like

BUG_ON(sme_active());

somewhere?


We can do that too.


Please don't do anything to cause Linux to crash if Xen is using SME 
itself, but leaving all of the PV guest unencrypted.


~Andrew

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

Re: [Xen-devel] [PATCH] xen: support 52 bit physical addresses in pv guests

2017-09-21 Thread Boris Ostrovsky 



On 09/21/2017 10:41 AM, Juergen Gross wrote:

On 21/09/17 16:14, Boris Ostrovsky wrote:



On 09/21/2017 04:01 AM, Juergen Gross wrote:

Physical addresses on processors supporting 5 level paging can be up to
52 bits wide. For a Xen pv guest running on such a machine those
physical addresses have to be supported in order to be able to use any
memory on the machine even if the guest itself does not support 5 level
paging.

So when reading/writing a MFN from/to a pte don't use the kernel's
PTE_PFN_MASK but a new XEN_PTE_MFN_MASK allowing full 40 bit wide MFNs.


full 52 bits?


The MFN mask is only 40 bits. This plus the 12 bits page offset are 52
bits of machine address width.


Oh, right --- frames, not addresses.







Signed-off-by: Juergen Gross 
---
   arch/x86/include/asm/xen/page.h | 11 ++-
   arch/x86/xen/mmu_pv.c   |  4 ++--
   2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/xen/page.h
b/arch/x86/include/asm/xen/page.h
index 07b6531813c4..bcb8b193c8d1 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -26,6 +26,15 @@ typedef struct xpaddr {
   phys_addr_t paddr;
   } xpaddr_t;
   +#ifdef CONFIG_X86_64
+#define XEN_PHYSICAL_MASK    ((1UL << 52) - 1)



SME is not supported for PV guests but for consistency (and in case sme
bit somehow gets set)
#define XEN_PHYSICAL_MASK    __sme_clr(((1UL << 52) - 1))


Hmm, really? Shouldn't we rather add something like

BUG_ON(sme_active());

somewhere?


We can do that too.




But the real question that I have is whether this patch is sufficient.
We are trying to preserve more bits in mfn but then this mfn is used,
say, in pte_pfn_to_mfn() to build a pte. Can we be sure that the pte
won't be stripped of higher bits in native code (again, as an example,
native_make_pte()) because we are compiled with 5LEVEL?


native_make_pte() just encapsulates pte_t. It doesn't modify the value
of the pte at all.


It's an interface though and so we don't know what is (or can) happen 
under it.


(I also obviously meant "because we are *not* compiled with 5LEVEL")



Physical address bits are only ever masked away via PTE_PFN_MASK and I
haven't found any place where it is used for a MFN other than those I
touched in this patch.



OK.

-boris

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

Re: [Xen-devel] [PATCH] xen: support 52 bit physical addresses in pv guests

2017-09-21 Thread Juergen Gross 
On 21/09/17 16:14, Boris Ostrovsky wrote:
> 
> 
> On 09/21/2017 04:01 AM, Juergen Gross wrote:
>> Physical addresses on processors supporting 5 level paging can be up to
>> 52 bits wide. For a Xen pv guest running on such a machine those
>> physical addresses have to be supported in order to be able to use any
>> memory on the machine even if the guest itself does not support 5 level
>> paging.
>>
>> So when reading/writing a MFN from/to a pte don't use the kernel's
>> PTE_PFN_MASK but a new XEN_PTE_MFN_MASK allowing full 40 bit wide MFNs.
> 
> full 52 bits?

The MFN mask is only 40 bits. This plus the 12 bits page offset are 52
bits of machine address width.

> 
>>
>> Signed-off-by: Juergen Gross 
>> ---
>>   arch/x86/include/asm/xen/page.h | 11 ++-
>>   arch/x86/xen/mmu_pv.c   |  4 ++--
>>   2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/xen/page.h
>> b/arch/x86/include/asm/xen/page.h
>> index 07b6531813c4..bcb8b193c8d1 100644
>> --- a/arch/x86/include/asm/xen/page.h
>> +++ b/arch/x86/include/asm/xen/page.h
>> @@ -26,6 +26,15 @@ typedef struct xpaddr {
>>   phys_addr_t paddr;
>>   } xpaddr_t;
>>   +#ifdef CONFIG_X86_64
>> +#define XEN_PHYSICAL_MASK    ((1UL << 52) - 1)
> 
> 
> SME is not supported for PV guests but for consistency (and in case sme
> bit somehow gets set)
> #define XEN_PHYSICAL_MASK    __sme_clr(((1UL << 52) - 1))

Hmm, really? Shouldn't we rather add something like

BUG_ON(sme_active());

somewhere?

> But the real question that I have is whether this patch is sufficient.
> We are trying to preserve more bits in mfn but then this mfn is used,
> say, in pte_pfn_to_mfn() to build a pte. Can we be sure that the pte
> won't be stripped of higher bits in native code (again, as an example,
> native_make_pte()) because we are compiled with 5LEVEL?

native_make_pte() just encapsulates pte_t. It doesn't modify the value
of the pte at all.

Physical address bits are only ever masked away via PTE_PFN_MASK and I
haven't found any place where it is used for a MFN other than those I
touched in this patch.


Juergen

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

Re: [Xen-devel] [PATCH] xen: support 52 bit physical addresses in pv guests

2017-09-21 Thread Boris Ostrovsky 



On 09/21/2017 04:01 AM, Juergen Gross wrote:

Physical addresses on processors supporting 5 level paging can be up to
52 bits wide. For a Xen pv guest running on such a machine those
physical addresses have to be supported in order to be able to use any
memory on the machine even if the guest itself does not support 5 level
paging.

So when reading/writing a MFN from/to a pte don't use the kernel's
PTE_PFN_MASK but a new XEN_PTE_MFN_MASK allowing full 40 bit wide MFNs.


full 52 bits?



Signed-off-by: Juergen Gross 
---
  arch/x86/include/asm/xen/page.h | 11 ++-
  arch/x86/xen/mmu_pv.c   |  4 ++--
  2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
index 07b6531813c4..bcb8b193c8d1 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -26,6 +26,15 @@ typedef struct xpaddr {
phys_addr_t paddr;
  } xpaddr_t;
  
+#ifdef CONFIG_X86_64

+#define XEN_PHYSICAL_MASK  ((1UL << 52) - 1)



SME is not supported for PV guests but for consistency (and in case sme 
bit somehow gets set)

#define XEN_PHYSICAL_MASK   __sme_clr(((1UL << 52) - 1))

But the real question that I have is whether this patch is sufficient. 
We are trying to preserve more bits in mfn but then this mfn is used, 
say, in pte_pfn_to_mfn() to build a pte. Can we be sure that the pte 
won't be stripped of higher bits in native code (again, as an example, 
native_make_pte()) because we are compiled with 5LEVEL?


-boris




+#else
+#define XEN_PHYSICAL_MASK  __PHYSICAL_MASK
+#endif
+
+#define XEN_PTE_MFN_MASK   ((pteval_t)(((signed long)PAGE_MASK) & \
+   XEN_PHYSICAL_MASK))
+
  #define XMADDR(x) ((xmaddr_t) { .maddr = (x) })
  #define XPADDR(x) ((xpaddr_t) { .paddr = (x) })
  
@@ -277,7 +286,7 @@ static inline unsigned long bfn_to_local_pfn(unsigned long mfn)
  
  static inline unsigned long pte_mfn(pte_t pte)

  {
-   return (pte.pte & PTE_PFN_MASK) >> PAGE_SHIFT;
+   return (pte.pte & XEN_PTE_MFN_MASK) >> PAGE_SHIFT;
  }
  
  static inline pte_t mfn_pte(unsigned long page_nr, pgprot_t pgprot)

diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index 509f560bd0c6..958d36d776d9 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -315,7 +315,7 @@ void xen_ptep_modify_prot_commit(struct mm_struct *mm, 
unsigned long addr,
  static pteval_t pte_mfn_to_pfn(pteval_t val)
  {
if (val & _PAGE_PRESENT) {
-   unsigned long mfn = (val & PTE_PFN_MASK) >> PAGE_SHIFT;
+   unsigned long mfn = (val & XEN_PTE_MFN_MASK) >> PAGE_SHIFT;
unsigned long pfn = mfn_to_pfn(mfn);
  
  		pteval_t flags = val & PTE_FLAGS_MASK;

@@ -1740,7 +1740,7 @@ static unsigned long __init m2p(phys_addr_t maddr)
  {
phys_addr_t paddr;
  
-	maddr &= PTE_PFN_MASK;

+   maddr &= XEN_PTE_MFN_MASK;
paddr = mfn_to_pfn(maddr >> PAGE_SHIFT) << PAGE_SHIFT;
  
  	return paddr;




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

[Xen-devel] [PATCH] xen: support 52 bit physical addresses in pv guests

2017-09-21 Thread Juergen Gross 
Physical addresses on processors supporting 5 level paging can be up to
52 bits wide. For a Xen pv guest running on such a machine those
physical addresses have to be supported in order to be able to use any
memory on the machine even if the guest itself does not support 5 level
paging.

So when reading/writing a MFN from/to a pte don't use the kernel's
PTE_PFN_MASK but a new XEN_PTE_MFN_MASK allowing full 40 bit wide MFNs.

Signed-off-by: Juergen Gross 
---
 arch/x86/include/asm/xen/page.h | 11 ++-
 arch/x86/xen/mmu_pv.c   |  4 ++--
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
index 07b6531813c4..bcb8b193c8d1 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -26,6 +26,15 @@ typedef struct xpaddr {
phys_addr_t paddr;
 } xpaddr_t;
 
+#ifdef CONFIG_X86_64
+#define XEN_PHYSICAL_MASK  ((1UL << 52) - 1)
+#else
+#define XEN_PHYSICAL_MASK  __PHYSICAL_MASK
+#endif
+
+#define XEN_PTE_MFN_MASK   ((pteval_t)(((signed long)PAGE_MASK) & \
+   XEN_PHYSICAL_MASK))
+
 #define XMADDR(x)  ((xmaddr_t) { .maddr = (x) })
 #define XPADDR(x)  ((xpaddr_t) { .paddr = (x) })
 
@@ -277,7 +286,7 @@ static inline unsigned long bfn_to_local_pfn(unsigned long 
mfn)
 
 static inline unsigned long pte_mfn(pte_t pte)
 {
-   return (pte.pte & PTE_PFN_MASK) >> PAGE_SHIFT;
+   return (pte.pte & XEN_PTE_MFN_MASK) >> PAGE_SHIFT;
 }
 
 static inline pte_t mfn_pte(unsigned long page_nr, pgprot_t pgprot)
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index 509f560bd0c6..958d36d776d9 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -315,7 +315,7 @@ void xen_ptep_modify_prot_commit(struct mm_struct *mm, 
unsigned long addr,
 static pteval_t pte_mfn_to_pfn(pteval_t val)
 {
if (val & _PAGE_PRESENT) {
-   unsigned long mfn = (val & PTE_PFN_MASK) >> PAGE_SHIFT;
+   unsigned long mfn = (val & XEN_PTE_MFN_MASK) >> PAGE_SHIFT;
unsigned long pfn = mfn_to_pfn(mfn);
 
pteval_t flags = val & PTE_FLAGS_MASK;
@@ -1740,7 +1740,7 @@ static unsigned long __init m2p(phys_addr_t maddr)
 {
phys_addr_t paddr;
 
-   maddr &= PTE_PFN_MASK;
+   maddr &= XEN_PTE_MFN_MASK;
paddr = mfn_to_pfn(maddr >> PAGE_SHIFT) << PAGE_SHIFT;
 
return paddr;
-- 
2.12.3


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