diff mbox series

[XEN,v2,02/15] x86/monitor: guard altp2m usage

Message ID 01767c3f98a88999d4b8ed3ae742ad66a0921ba3.1715761386.git.Sergiy_Kibrik@epam.com (mailing list archive)
State New
Headers show
Series x86: make cpu virtualization support configurable | expand

Commit Message

Sergiy Kibrik May 15, 2024, 9:01 a.m. UTC
Explicitly check whether altp2m is on for domain when getting altp2m index.
If explicit call to altp2m_active() always returns false, DCE will remove
call to altp2m_vcpu_idx().

The puspose of that is later to be able to disable altp2m support and
exclude its code from the build completely, when not supported by target
platform (as of now it's supported for VT-d only).

Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: Tamas K Lengyel <tamas@tklengyel.com>
CC: Jan Beulich <jbeulich@suse.com>
---
changes in v2:
 - patch description changed, removed VMX mentioning
 - guard by altp2m_active() instead of hvm_altp2m_supported()
---
 xen/arch/x86/hvm/monitor.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Stefano Stabellini May 16, 2024, 12:29 a.m. UTC | #1
On Wed, 15 May 2024, Sergiy Kibrik wrote:
> Explicitly check whether altp2m is on for domain when getting altp2m index.
> If explicit call to altp2m_active() always returns false, DCE will remove
> call to altp2m_vcpu_idx().
> 
> The puspose of that is later to be able to disable altp2m support and
> exclude its code from the build completely, when not supported by target
> platform (as of now it's supported for VT-d only).
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: Tamas K Lengyel <tamas@tklengyel.com>
> CC: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> changes in v2:
>  - patch description changed, removed VMX mentioning
>  - guard by altp2m_active() instead of hvm_altp2m_supported()
> ---
>  xen/arch/x86/hvm/monitor.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
> index 2a8ff07ec9..74621000b2 100644
> --- a/xen/arch/x86/hvm/monitor.c
> +++ b/xen/arch/x86/hvm/monitor.c
> @@ -262,6 +262,8 @@ bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
>      struct vcpu *curr = current;
>      vm_event_request_t req = {};
>      paddr_t gpa = (gfn_to_gaddr(gfn) | (gla & ~PAGE_MASK));
> +    unsigned int altp2m_idx = altp2m_active(curr->domain) ?
> +                              altp2m_vcpu_idx(curr) : 0;
>      int rc;
>  
>      ASSERT(curr->arch.vm_event->send_event);
> @@ -270,7 +272,7 @@ bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
>       * p2m_get_mem_access() can fail from a invalid MFN and return -ESRCH
>       * in which case access must be restricted.
>       */
> -    rc = p2m_get_mem_access(curr->domain, gfn, &access, altp2m_vcpu_idx(curr));
> +    rc = p2m_get_mem_access(curr->domain, gfn, &access, altp2m_idx);
>  
>      if ( rc == -ESRCH )
>          access = XENMEM_access_n;
> -- 
> 2.25.1
>
Jan Beulich May 16, 2024, 10:20 a.m. UTC | #2
On 15.05.2024 11:01, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/hvm/monitor.c
> +++ b/xen/arch/x86/hvm/monitor.c
> @@ -262,6 +262,8 @@ bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
>      struct vcpu *curr = current;
>      vm_event_request_t req = {};
>      paddr_t gpa = (gfn_to_gaddr(gfn) | (gla & ~PAGE_MASK));
> +    unsigned int altp2m_idx = altp2m_active(curr->domain) ?
> +                              altp2m_vcpu_idx(curr) : 0;
>      int rc;
>  
>      ASSERT(curr->arch.vm_event->send_event);
> @@ -270,7 +272,7 @@ bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
>       * p2m_get_mem_access() can fail from a invalid MFN and return -ESRCH
>       * in which case access must be restricted.
>       */
> -    rc = p2m_get_mem_access(curr->domain, gfn, &access, altp2m_vcpu_idx(curr));
> +    rc = p2m_get_mem_access(curr->domain, gfn, &access, altp2m_idx);

I think something like this wants taking care of by the function itself.
asm/altp2m.h already has certain stubs (the conditional of which I expect
a later change will switch to CONFIG_ALTP2M); you'd need to just add one
more. Then the other (and any future) users of the function would be
covered as well.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index 2a8ff07ec9..74621000b2 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -262,6 +262,8 @@  bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
     struct vcpu *curr = current;
     vm_event_request_t req = {};
     paddr_t gpa = (gfn_to_gaddr(gfn) | (gla & ~PAGE_MASK));
+    unsigned int altp2m_idx = altp2m_active(curr->domain) ?
+                              altp2m_vcpu_idx(curr) : 0;
     int rc;
 
     ASSERT(curr->arch.vm_event->send_event);
@@ -270,7 +272,7 @@  bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
      * p2m_get_mem_access() can fail from a invalid MFN and return -ESRCH
      * in which case access must be restricted.
      */
-    rc = p2m_get_mem_access(curr->domain, gfn, &access, altp2m_vcpu_idx(curr));
+    rc = p2m_get_mem_access(curr->domain, gfn, &access, altp2m_idx);
 
     if ( rc == -ESRCH )
         access = XENMEM_access_n;