[PATCH v10 49/59] i386/xen: handle HVMOP_get_param

David Woodhouse posted 59 patches 2 years, 3 months ago
There is a newer version of this series
[PATCH v10 49/59] i386/xen: handle HVMOP_get_param
Posted by David Woodhouse 2 years, 3 months ago
From: Joao Martins <joao.m.martins@oracle.com>

Which is used to fetch xenstore PFN and port to be used
by the guest. This is preallocated by the toolstack when
guest will just read those and use it straight away.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 target/i386/kvm/xen-emu.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/target/i386/kvm/xen-emu.c b/target/i386/kvm/xen-emu.c
index 75e62bc02f..3d6ea7ca98 100644
--- a/target/i386/kvm/xen-emu.c
+++ b/target/i386/kvm/xen-emu.c
@@ -762,6 +762,42 @@ out:
     return true;
 }
 
+static bool handle_get_param(struct kvm_xen_exit *exit, X86CPU *cpu,
+                             uint64_t arg)
+{
+    CPUState *cs = CPU(cpu);
+    struct xen_hvm_param hp;
+    int err = 0;
+
+    /* No need for 32/64 compat handling */
+    qemu_build_assert(sizeof(hp) == 16);
+
+    if (kvm_copy_from_gva(cs, arg, &hp, sizeof(hp))) {
+        err = -EFAULT;
+        goto out;
+    }
+
+    if (hp.domid != DOMID_SELF && hp.domid != xen_domid) {
+        err = -ESRCH;
+        goto out;
+    }
+
+    switch (hp.index) {
+    case HVM_PARAM_STORE_PFN:
+        hp.value = XEN_SPECIAL_PFN(XENSTORE);
+        break;
+    default:
+        return false;
+    }
+
+    if (kvm_copy_to_gva(cs, arg, &hp, sizeof(hp))) {
+        err = -EFAULT;
+    }
+out:
+    exit->u.hcall.result = err;
+    return true;
+}
+
 static int kvm_xen_hcall_evtchn_upcall_vector(struct kvm_xen_exit *exit,
                                               X86CPU *cpu, uint64_t arg)
 {
@@ -806,6 +842,9 @@ static bool kvm_xen_hcall_hvm_op(struct kvm_xen_exit *exit, X86CPU *cpu,
     case HVMOP_set_param:
         return handle_set_param(exit, cpu, arg);
 
+    case HVMOP_get_param:
+        return handle_get_param(exit, cpu, arg);
+
     default:
         return false;
     }
-- 
2.39.0
Re: [PATCH v10 49/59] i386/xen: handle HVMOP_get_param
Posted by Paul Durrant 2 years, 2 months ago
On 01/02/2023 14:31, David Woodhouse wrote:
> From: Joao Martins <joao.m.martins@oracle.com>
> 
> Which is used to fetch xenstore PFN and port to be used
> by the guest. This is preallocated by the toolstack when
> guest will just read those and use it straight away.
> 
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>   target/i386/kvm/xen-emu.c | 39 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
> 
> diff --git a/target/i386/kvm/xen-emu.c b/target/i386/kvm/xen-emu.c
> index 75e62bc02f..3d6ea7ca98 100644
> --- a/target/i386/kvm/xen-emu.c
> +++ b/target/i386/kvm/xen-emu.c
> @@ -762,6 +762,42 @@ out:
>       return true;
>   }
>   
> +static bool handle_get_param(struct kvm_xen_exit *exit, X86CPU *cpu,
> +                             uint64_t arg)
> +{
> +    CPUState *cs = CPU(cpu);
> +    struct xen_hvm_param hp;
> +    int err = 0;
> +
> +    /* No need for 32/64 compat handling */
> +    qemu_build_assert(sizeof(hp) == 16);
> +
> +    if (kvm_copy_from_gva(cs, arg, &hp, sizeof(hp))) {
> +        err = -EFAULT;
> +        goto out;
> +    }
> +
> +    if (hp.domid != DOMID_SELF && hp.domid != xen_domid) {
> +        err = -ESRCH;
> +        goto out;
> +    }
> +
> +    switch (hp.index) {
> +    case HVM_PARAM_STORE_PFN:
> +        hp.value = XEN_SPECIAL_PFN(XENSTORE);

Reviewed-by: Paul Durrant <paul@xen.org>

... but this reminds me... I don't think you have code to seed the grant 
table in any of the patches. It is guest ABI that the XenStore PFN is in 
entry 1 of the grant table.

> +        break;
> +    default:
> +        return false;
> +    }
> +
> +    if (kvm_copy_to_gva(cs, arg, &hp, sizeof(hp))) {
> +        err = -EFAULT;
> +    }
> +out:
> +    exit->u.hcall.result = err;
> +    return true;
> +}
> +
>   static int kvm_xen_hcall_evtchn_upcall_vector(struct kvm_xen_exit *exit,
>                                                 X86CPU *cpu, uint64_t arg)
>   {
> @@ -806,6 +842,9 @@ static bool kvm_xen_hcall_hvm_op(struct kvm_xen_exit *exit, X86CPU *cpu,
>       case HVMOP_set_param:
>           return handle_set_param(exit, cpu, arg);
>   
> +    case HVMOP_get_param:
> +        return handle_get_param(exit, cpu, arg);
> +
>       default:
>           return false;
>       }
Re: [PATCH v10 49/59] i386/xen: handle HVMOP_get_param
Posted by David Woodhouse 2 years, 2 months ago

On 14 February 2023 16:47:13 CET, Paul Durrant <xadimgnik@gmail.com> wrote:
>On 01/02/2023 14:31, David Woodhouse wrote:
>> From: Joao Martins <joao.m.martins@oracle.com>
>> 
>> Which is used to fetch xenstore PFN and port to be used
>> by the guest. This is preallocated by the toolstack when
>> guest will just read those and use it straight away.
>> 
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
>> ---
>>   target/i386/kvm/xen-emu.c | 39 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>> 
>> diff --git a/target/i386/kvm/xen-emu.c b/target/i386/kvm/xen-emu.c
>> index 75e62bc02f..3d6ea7ca98 100644
>> --- a/target/i386/kvm/xen-emu.c
>> +++ b/target/i386/kvm/xen-emu.c
>> @@ -762,6 +762,42 @@ out:
>>       return true;
>>   }
>>   +static bool handle_get_param(struct kvm_xen_exit *exit, X86CPU *cpu,
>> +                             uint64_t arg)
>> +{
>> +    CPUState *cs = CPU(cpu);
>> +    struct xen_hvm_param hp;
>> +    int err = 0;
>> +
>> +    /* No need for 32/64 compat handling */
>> +    qemu_build_assert(sizeof(hp) == 16);
>> +
>> +    if (kvm_copy_from_gva(cs, arg, &hp, sizeof(hp))) {
>> +        err = -EFAULT;
>> +        goto out;
>> +    }
>> +
>> +    if (hp.domid != DOMID_SELF && hp.domid != xen_domid) {
>> +        err = -ESRCH;
>> +        goto out;
>> +    }
>> +
>> +    switch (hp.index) {
>> +    case HVM_PARAM_STORE_PFN:
>> +        hp.value = XEN_SPECIAL_PFN(XENSTORE);
>
>Reviewed-by: Paul Durrant <paul@xen.org>
>
>... but this reminds me... I don't think you have code to seed the grant table in any of the patches. It is guest ABI that the XenStore PFN is in entry 1 of the grant table.

It's in there somewhere, perhaps in "phase 2" where we actually add a real XenStore rather than this sequence of 59 (and counting).

I even made XenStore map the grant (not that it actually *uses* the address it gets back).