[PATCH] x86/HVM: Merge HVM and PVH hypercall tables

[PATCH] x86/HVM: Merge HVM and PVH hypercall tables

[Boris Ostrovsky]

The tables are almost identical and therefore there is little reason to
keep both sets.

PVH needs 3 extra hypercalls:
* mmuext_op. PVH uses MMUEXT_TLB_FLUSH_MULTI and MMUEXT_INVLPG_MULTI to
  optimize TLB flushing. Since HVMlite guests may decide to use them as
  well we can allow these two commands for all guests in an HVM container.
* platform_op. These are only available to privileged domains. We will
  (eventually) have privileged HVMlite guests and therefore shouldn't
  limit this to PVH only.
* xenpmu_op. any guest with !has_vlapic() (i.e. PV, PVH and HVMlite)
  should be able to use it.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---

This patch is intended to go on top of Roger's HVMlite series

 xen/arch/x86/hvm/hvm.c |   65 ++++++++++-------------------------------------
 xen/arch/x86/mm.c      |    9 ++++++
 2 files changed, 23 insertions(+), 51 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 08cef1f..38293d1 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5173,12 +5173,21 @@ static hvm_hypercall_t *const hvm_hypercall64_table[NR_hypercalls] = {
     HYPERCALL(sysctl),
     HYPERCALL(domctl),
     HYPERCALL(tmem_op),
+    HYPERCALL(platform_op),
+    HYPERCALL(mmuext_op),
+    HYPERCALL(xenpmu_op),
     [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
 };
 
 #define COMPAT_CALL(x)                                        \
     [ __HYPERVISOR_ ## x ] = (hvm_hypercall_t *) compat_ ## x
 
+extern int compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(void) cmp_uops,
+                            unsigned int count,
+                            XEN_GUEST_HANDLE_PARAM(uint) pdone,
+                            unsigned int foreigndom);
+extern int compat_platform_op(XEN_GUEST_HANDLE_PARAM(void) u_xenpf_op);
+
 static hvm_hypercall_t *const hvm_hypercall32_table[NR_hypercalls] = {
     [ __HYPERVISOR_memory_op ] = (hvm_hypercall_t *)hvm_memory_op_compat32,
     [ __HYPERVISOR_grant_table_op ] = (hvm_hypercall_t *)hvm_grant_table_op_compat32,
@@ -5194,48 +5203,8 @@ static hvm_hypercall_t *const hvm_hypercall32_table[NR_hypercalls] = {
     HYPERCALL(sysctl),
     HYPERCALL(domctl),
     HYPERCALL(tmem_op),
-    [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
-};
-
-static hvm_hypercall_t *const pvh_hypercall64_table[NR_hypercalls] = {
-    HYPERCALL(platform_op),
-    HYPERCALL(memory_op),
-    HYPERCALL(xen_version),
-    HYPERCALL(console_io),
-    [ __HYPERVISOR_grant_table_op ]  = (hvm_hypercall_t *)hvm_grant_table_op,
-    HYPERCALL(vcpu_op),
-    HYPERCALL(mmuext_op),
-    HYPERCALL(xsm_op),
-    HYPERCALL(sched_op),
-    HYPERCALL(event_channel_op),
-    [ __HYPERVISOR_physdev_op ]      = (hvm_hypercall_t *)hvm_physdev_op,
-    HYPERCALL(hvm_op),
-    HYPERCALL(sysctl),
-    HYPERCALL(domctl),
-    HYPERCALL(xenpmu_op),
-    [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
-};
-
-extern int compat_mmuext_op(XEN_GUEST_HANDLE_PARAM(void) cmp_uops,
-                            unsigned int count,
-                            XEN_GUEST_HANDLE_PARAM(uint) pdone,
-                            unsigned int foreigndom);
-static hvm_hypercall_t *const pvh_hypercall32_table[NR_hypercalls] = {
-    HYPERCALL(platform_op),
-    COMPAT_CALL(memory_op),
-    HYPERCALL(xen_version),
-    HYPERCALL(console_io),
-    [ __HYPERVISOR_grant_table_op ]  =
-        (hvm_hypercall_t *)hvm_grant_table_op_compat32,
-    COMPAT_CALL(vcpu_op),
+    COMPAT_CALL(platform_op),
     COMPAT_CALL(mmuext_op),
-    HYPERCALL(xsm_op),
-    COMPAT_CALL(sched_op),
-    HYPERCALL(event_channel_op),
-    [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op_compat32,
-    HYPERCALL(hvm_op),
-    HYPERCALL(sysctl),
-    HYPERCALL(domctl),
     HYPERCALL(xenpmu_op),
     [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
 };
@@ -5269,9 +5238,7 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
     if ( (eax & 0x80000000) && is_viridian_domain(currd) )
         return viridian_hypercall(regs);
 
-    if ( (eax >= NR_hypercalls) ||
-         !(is_pvh_domain(currd) ? pvh_hypercall32_table[eax]
-                                : hvm_hypercall32_table[eax]) )
+    if ( (eax >= NR_hypercalls) || !hvm_hypercall32_table[eax] )
     {
         regs->eax = -ENOSYS;
         return HVM_HCALL_completed;
@@ -5305,9 +5272,8 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
 #endif
 
         curr->arch.hvm_vcpu.hcall_64bit = 1;
-        regs->rax = (is_pvh_domain(currd)
-                     ? pvh_hypercall64_table
-                     : hvm_hypercall64_table)[eax](rdi, rsi, rdx, r10, r8, r9);
+        regs->rax = hvm_hypercall64_table[eax](rdi, rsi, rdx, r10, r8, r9);
+
         curr->arch.hvm_vcpu.hcall_64bit = 0;
 
 #ifndef NDEBUG
@@ -5351,10 +5317,7 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
         }
 #endif
 
-        regs->_eax = (is_pvh_vcpu(curr)
-                      ? pvh_hypercall32_table
-                      : hvm_hypercall32_table)[eax](ebx, ecx, edx,
-                                                    esi, edi, ebp);
+        regs->_eax = hvm_hypercall32_table[eax](ebx, ecx, edx, esi, edi, ebp);
 
 #ifndef NDEBUG
         if ( !curr->arch.hvm_vcpu.hcall_preempted )
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 92df36f..f3700f5 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3019,6 +3019,15 @@ long do_mmuext_op(
             break;
         }
 
+        if ( has_hvm_container_domain(d) &&
+             (op.cmd != MMUEXT_TLB_FLUSH_MULTI) &&
+             (op.cmd != MMUEXT_INVLPG_MULTI) )
+        {
+            MEM_LOG("Unsupported mmuext cmd %u", op.cmd);
+            rc = -ENOSYS;
+            break;
+        }
+
         okay = 1;
 
         switch ( op.cmd )
-- 
1.7.1

Re: [PATCH] x86/HVM: Merge HVM and PVH hypercall tables

[Jan Beulich]

>>> On 08.12.15 at 15:20, <boris.ostrovsky@oracle.com> wrote:
> The tables are almost identical and therefore there is little reason to
> keep both sets.
> 
> PVH needs 3 extra hypercalls:
> * mmuext_op. PVH uses MMUEXT_TLB_FLUSH_MULTI and MMUEXT_INVLPG_MULTI to
>   optimize TLB flushing. Since HVMlite guests may decide to use them as
>   well we can allow these two commands for all guests in an HVM container.

I must be missing something here: Especially for the INVLPG variant
I can't see what use it could be for a PVH guest, as it necessarily
would act on a different address space (the other one may have at
least some effect due to hvm_flush_guest_tlbs()).

And then, if those two really are meant to be enabled, why would
their _LOCAL and _ALL counterparts not be? And similarly,
MMUEXT_FLUSH_CACHE{,_GLOBAL} may then be valid to expose.

Wasn't it much rather that PVH Dom0 needed e.g. MMUEXT_PIN_Ln_TABLE
to deal with foreign guests' page tables?

Jan

Re: [PATCH] x86/HVM: Merge HVM and PVH hypercall tables

[Boris Ostrovsky]

On 12/10/2015 07:30 AM, Jan Beulich wrote:
>>>> On 08.12.15 at 15:20, <boris.ostrovsky@oracle.com> wrote:
>> The tables are almost identical and therefore there is little reason to
>> keep both sets.
>>
>> PVH needs 3 extra hypercalls:
>> * mmuext_op. PVH uses MMUEXT_TLB_FLUSH_MULTI and MMUEXT_INVLPG_MULTI to
>>    optimize TLB flushing. Since HVMlite guests may decide to use them as
>>    well we can allow these two commands for all guests in an HVM container.
> I must be missing something here: Especially for the INVLPG variant
> I can't see what use it could be for a PVH guest, as it necessarily
> would act on a different address space (the other one may have at
> least some effect due to hvm_flush_guest_tlbs()).

This is done out of xen_flush_tlb_others(), which is what PVH guests use.

And yes --- there indeed seems to be little reason to do that. But it is 
there now so I am not sure we can make this not work anymore for PVH guests.

>
> And then, if those two really are meant to be enabled, why would
> their _LOCAL and _ALL counterparts not be? And similarly,
> MMUEXT_FLUSH_CACHE{,_GLOBAL} may then be valid to expose.

This is only used by PVH guests as optimization (see comment in 
xen_init_mmu_ops()). So there is no need to do a hypercall for LOCAL 
operations. For ALL/GLOBAL --- maybe we should allow those too, even 
though they are not currently used (in Linux).

(In principle we could allow LOCAL ones too. Assuming this all is needed 
at all)

>
> Wasn't it much rather that PVH Dom0 needed e.g. MMUEXT_PIN_Ln_TABLE
> to deal with foreign guests' page tables?

That I haven't considered.

Especially given that PVH dom0 is not booting for me, as I just found out:

...
(XEN) d0v0 EPT violation 0x1aa (-w-/r-x) gpa 0x000000c0008116 mfn 
0xc0008 type 5
(XEN) d0v0 Walking EPT tables for GFN c0008:
(XEN) d0v0  epte 800000082bf50007
(XEN) d0v0  epte 800000082bf19007
(XEN) d0v0  epte 800000043c6f9007
(XEN) d0v0  epte 80500000c0008805
(XEN) d0v0  --- GLA 0xffffc90020008116
(XEN) domain_crash called from vmx.c:2816
(XEN) Domain 0 (vcpu#0) crashed on cpu#0:
(XEN) ----[ Xen-4.7-unstable  x86_64  debug=y  Tainted:    C ]----
(XEN) CPU:    0
(XEN) RIP:    0010:[<ffffffff816150dc>]
(XEN) RFLAGS: 0000000000010046   CONTEXT: hvm guest (d0v0)
(XEN) rax: 000000000000001d   rbx: 0000000000000000   rcx: ffff88014700f9b8
(XEN) rdx: 00000000000000ff   rsi: 0000000000000000   rdi: 0000000000000000
(XEN) rbp: ffff88014700fa18   rsp: ffff88014700f9e8   r8: ffff88014700f9c0
(XEN) r9:  000000000000001d   r10: ffffffff8189c7f0   r11: 0000000000000000
(XEN) r12: ffffc90020008000   r13: ffffc90020008116   r14: 0000000000000002
(XEN) r15: 000000000000001d   cr0: 0000000080050033   cr4: 00000000000406f0
(XEN) cr3: 0000000001c0e000   cr2: 0000000000000000
(XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: 0000   cs: 0010
(XEN) Guest stack trace from rsp=ffff88014700f9e8:
(XEN)   Fault while accessing guest memory.
(XEN) Hardware Dom0 crashed: rebooting machine in 5 seconds.


We haven't been running regression tests for PVH dom0 so I don't know 
how long this has been broken.

-boris

Re: [PATCH] x86/HVM: Merge HVM and PVH hypercall tables

[Jan Beulich]

>>> On 10.12.15 at 17:53, <boris.ostrovsky@oracle.com> wrote:
> On 12/10/2015 07:30 AM, Jan Beulich wrote:
>>>>> On 08.12.15 at 15:20, <boris.ostrovsky@oracle.com> wrote:
>>> The tables are almost identical and therefore there is little reason to
>>> keep both sets.
>>>
>>> PVH needs 3 extra hypercalls:
>>> * mmuext_op. PVH uses MMUEXT_TLB_FLUSH_MULTI and MMUEXT_INVLPG_MULTI to
>>>    optimize TLB flushing. Since HVMlite guests may decide to use them as
>>>    well we can allow these two commands for all guests in an HVM container.
>> I must be missing something here: Especially for the INVLPG variant
>> I can't see what use it could be for a PVH guest, as it necessarily
>> would act on a different address space (the other one may have at
>> least some effect due to hvm_flush_guest_tlbs()).
> 
> This is done out of xen_flush_tlb_others(), which is what PVH guests use.
> 
> And yes --- there indeed seems to be little reason to do that. But it is 
> there now so I am not sure we can make this not work anymore for PVH guests.

PVH is experimental, so we certainly can. I'm pretty determined that
we shouldn't expose functionality to PVH (or HVMlite) that can't be
actually useful to it. I think it was a mistake to allow PVH general
access to all MMUEXT ops (I thought this had been discussed, but I
can't seem to find that discussion going back to v12 of the PVH series,
and I can't even spot the particular patch earlier than in v12).

Jan

Re: [PATCH] x86/HVM: Merge HVM and PVH hypercall tables

[Boris Ostrovsky]

On 12/10/2015 11:53 AM, Boris Ostrovsky wrote:
>
>
>
> Especially given that PVH dom0 is not booting for me, as I just found 
> out:
>
> ...
> (XEN) d0v0 EPT violation 0x1aa (-w-/r-x) gpa 0x000000c0008116 mfn 
> 0xc0008 type 5
> (XEN) d0v0 Walking EPT tables for GFN c0008:
> (XEN) d0v0  epte 800000082bf50007
> (XEN) d0v0  epte 800000082bf19007
> (XEN) d0v0  epte 800000043c6f9007
> (XEN) d0v0  epte 80500000c0008805
> (XEN) d0v0  --- GLA 0xffffc90020008116
> (XEN) domain_crash called from vmx.c:2816
> (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
> (XEN) ----[ Xen-4.7-unstable  x86_64  debug=y  Tainted:    C ]----
> (XEN) CPU:    0
> (XEN) RIP:    0010:[<ffffffff816150dc>]
> (XEN) RFLAGS: 0000000000010046   CONTEXT: hvm guest (d0v0)
> (XEN) rax: 000000000000001d   rbx: 0000000000000000   rcx: 
> ffff88014700f9b8
> (XEN) rdx: 00000000000000ff   rsi: 0000000000000000   rdi: 
> 0000000000000000
> (XEN) rbp: ffff88014700fa18   rsp: ffff88014700f9e8   r8: 
> ffff88014700f9c0
> (XEN) r9:  000000000000001d   r10: ffffffff8189c7f0   r11: 
> 0000000000000000
> (XEN) r12: ffffc90020008000   r13: ffffc90020008116   r14: 
> 0000000000000002
> (XEN) r15: 000000000000001d   cr0: 0000000080050033   cr4: 
> 00000000000406f0
> (XEN) cr3: 0000000001c0e000   cr2: 0000000000000000
> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: 0000   cs: 0010
> (XEN) Guest stack trace from rsp=ffff88014700f9e8:
> (XEN)   Fault while accessing guest memory.
> (XEN) Hardware Dom0 crashed: rebooting machine in 5 seconds.
>
>
> We haven't been running regression tests for PVH dom0 so I don't know 
> how long this has been broken.


This is broken by 9256f66c1606 ("x86/PCI: intercept all PV Dom0 MMCFG 
writes").

-boris

Re: [PATCH] x86/HVM: Merge HVM and PVH hypercall tables

[Jan Beulich]

>>> On 11.12.15 at 17:50, <boris.ostrovsky@oracle.com> wrote:
> On 12/10/2015 11:53 AM, Boris Ostrovsky wrote:
>> Especially given that PVH dom0 is not booting for me, as I just found 
>> out:
>>
>> ...
>> (XEN) d0v0 EPT violation 0x1aa (-w-/r-x) gpa 0x000000c0008116 mfn 
>> 0xc0008 type 5
>> (XEN) d0v0 Walking EPT tables for GFN c0008:
>> (XEN) d0v0  epte 800000082bf50007
>> (XEN) d0v0  epte 800000082bf19007
>> (XEN) d0v0  epte 800000043c6f9007
>> (XEN) d0v0  epte 80500000c0008805
>> (XEN) d0v0  --- GLA 0xffffc90020008116
>> (XEN) domain_crash called from vmx.c:2816
>> (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
>> (XEN) ----[ Xen-4.7-unstable  x86_64  debug=y  Tainted:    C ]----
>> (XEN) CPU:    0
>> (XEN) RIP:    0010:[<ffffffff816150dc>]
>> (XEN) RFLAGS: 0000000000010046   CONTEXT: hvm guest (d0v0)
>> (XEN) rax: 000000000000001d   rbx: 0000000000000000   rcx: 
>> ffff88014700f9b8
>> (XEN) rdx: 00000000000000ff   rsi: 0000000000000000   rdi: 
>> 0000000000000000
>> (XEN) rbp: ffff88014700fa18   rsp: ffff88014700f9e8   r8: 
>> ffff88014700f9c0
>> (XEN) r9:  000000000000001d   r10: ffffffff8189c7f0   r11: 
>> 0000000000000000
>> (XEN) r12: ffffc90020008000   r13: ffffc90020008116   r14: 
>> 0000000000000002
>> (XEN) r15: 000000000000001d   cr0: 0000000080050033   cr4: 
>> 00000000000406f0
>> (XEN) cr3: 0000000001c0e000   cr2: 0000000000000000
>> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: 0000   cs: 0010
>> (XEN) Guest stack trace from rsp=ffff88014700f9e8:
>> (XEN)   Fault while accessing guest memory.
>> (XEN) Hardware Dom0 crashed: rebooting machine in 5 seconds.
>>
>>
>> We haven't been running regression tests for PVH dom0 so I don't know 
>> how long this has been broken.
> 
> 
> This is broken by 9256f66c1606 ("x86/PCI: intercept all PV Dom0 MMCFG 
> writes").

Well, I can't find any hookup of the write emulation logic for PVH
at all in ept_handle_violation() or hvm_hap_nested_page_fault(),
i.e. it looks to me as if this was broken already before, just that
for the limited set of devices that had their MMCFG space marked
r/o this went unnoticed (iow perhaps a missing FIXME annotation).
I'll see to find time to look into this, but I can't really predict when
I might get around to it.

Jan

Re: [PATCH] x86/HVM: Merge HVM and PVH hypercall tables

[Boris Ostrovsky]

On 12/14/2015 02:51 AM, Jan Beulich wrote:
>>>> On 11.12.15 at 17:50, <boris.ostrovsky@oracle.com> wrote:
>> On 12/10/2015 11:53 AM, Boris Ostrovsky wrote:
>>> Especially given that PVH dom0 is not booting for me, as I just found
>>> out:
>>>
>>> ...
>>> (XEN) d0v0 EPT violation 0x1aa (-w-/r-x) gpa 0x000000c0008116 mfn
>>> 0xc0008 type 5
>>> (XEN) d0v0 Walking EPT tables for GFN c0008:
>>> (XEN) d0v0  epte 800000082bf50007
>>> (XEN) d0v0  epte 800000082bf19007
>>> (XEN) d0v0  epte 800000043c6f9007
>>> (XEN) d0v0  epte 80500000c0008805
>>> (XEN) d0v0  --- GLA 0xffffc90020008116
>>> (XEN) domain_crash called from vmx.c:2816
>>> (XEN) Domain 0 (vcpu#0) crashed on cpu#0:
>>> (XEN) ----[ Xen-4.7-unstable  x86_64  debug=y  Tainted:    C ]----
>>> (XEN) CPU:    0
>>> (XEN) RIP:    0010:[<ffffffff816150dc>]
>>> (XEN) RFLAGS: 0000000000010046   CONTEXT: hvm guest (d0v0)
>>> (XEN) rax: 000000000000001d   rbx: 0000000000000000   rcx:
>>> ffff88014700f9b8
>>> (XEN) rdx: 00000000000000ff   rsi: 0000000000000000   rdi:
>>> 0000000000000000
>>> (XEN) rbp: ffff88014700fa18   rsp: ffff88014700f9e8   r8:
>>> ffff88014700f9c0
>>> (XEN) r9:  000000000000001d   r10: ffffffff8189c7f0   r11:
>>> 0000000000000000
>>> (XEN) r12: ffffc90020008000   r13: ffffc90020008116   r14:
>>> 0000000000000002
>>> (XEN) r15: 000000000000001d   cr0: 0000000080050033   cr4:
>>> 00000000000406f0
>>> (XEN) cr3: 0000000001c0e000   cr2: 0000000000000000
>>> (XEN) ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: 0000   cs: 0010
>>> (XEN) Guest stack trace from rsp=ffff88014700f9e8:
>>> (XEN)   Fault while accessing guest memory.
>>> (XEN) Hardware Dom0 crashed: rebooting machine in 5 seconds.
>>>
>>>
>>> We haven't been running regression tests for PVH dom0 so I don't know
>>> how long this has been broken.
>>
>> This is broken by 9256f66c1606 ("x86/PCI: intercept all PV Dom0 MMCFG
>> writes").
> Well, I can't find any hookup of the write emulation logic for PVH
> at all in ept_handle_violation() or hvm_hap_nested_page_fault(),
> i.e. it looks to me as if this was broken already before, just that
> for the limited set of devices that had their MMCFG space marked
> r/o this went unnoticed (iow perhaps a missing FIXME annotation).
> I'll see to find time to look into this, but I can't really predict when
> I might get around to it.

I added handling for MMCFG to ept_handle_violation() and that indeed 
fixed it (but it should really go to hvm_hap_nested_page_fault() since 
we will want AMD support for that as well).

I'll try to come up with a patch in the next day or two.

-boris