Re: [RFC PATCH 12/21] i386/xen: set shared_info page - Philippe Mathieu-Daudé

From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: David Woodhouse <dwmw2@infradead.org>, qemu-devel@nongnu.org
Cc: Paolo Bonzini <pbonzini@redhat.com>, Paul Durrant <paul@xen.org>,
	Joao Martins <joao.m.martins@oracle.com>,
	Ankur Arora <ankur.a.arora@oracle.com>
Subject: Re: [RFC PATCH 12/21] i386/xen: set shared_info page
Date: Mon, 5 Dec 2022 23:17:49 +0100	[thread overview]
Message-ID: <cbb522e1-3d8d-5332-7ac8-c0e054be33e2@linaro.org> (raw)
In-Reply-To: <20221205173137.607044-13-dwmw2@infradead.org>

On 5/12/22 18:31, David Woodhouse wrote:
> From: Joao Martins <joao.m.martins@oracle.com>
> 
> This is done by implementing HYPERVISOR_memory_op specifically
> XENMEM_add_to_physmap with space XENMAPSPACE_shared_info. While
> Xen removes the page with its own, we instead use the gfn passed
> by the guest.
> 
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>   accel/kvm/kvm-all.c      |  6 ++++
>   include/hw/core/cpu.h    |  2 ++
>   include/sysemu/kvm.h     |  2 ++
>   include/sysemu/kvm_int.h |  3 ++
>   target/i386/cpu.h        |  8 ++++++
>   target/i386/trace-events |  1 +
>   target/i386/xen-proto.h  | 19 +++++++++++++
>   target/i386/xen.c        | 61 ++++++++++++++++++++++++++++++++++++++++
>   8 files changed, 102 insertions(+)
>   create mode 100644 target/i386/xen-proto.h

> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 8830546121..e57b693528 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -443,6 +443,8 @@ struct CPUState {
>   
>       /* track IOMMUs whose translations we've cached in the TCG TLB */
>       GArray *iommu_notifiers;
> +
> +    struct XenState *xen_state;

Since you define a type definition below, use it.

>   };
>   
>   typedef QTAILQ_HEAD(CPUTailQ, CPUState) CPUTailQ;
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index e9a97eda8c..8e882fbe96 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -582,4 +582,6 @@ bool kvm_arch_cpu_check_are_resettable(void);
>   bool kvm_dirty_ring_enabled(void);
>   
>   uint32_t kvm_dirty_ring_size(void);
> +
> +struct XenState *kvm_get_xen_state(KVMState *s);

Ditto.

>   #endif
> diff --git a/include/sysemu/kvm_int.h b/include/sysemu/kvm_int.h
> index 3b4adcdc10..0d89cfe273 100644
> --- a/include/sysemu/kvm_int.h
> +++ b/include/sysemu/kvm_int.h
> @@ -110,6 +110,9 @@ struct KVMState
>       struct KVMDirtyRingReaper reaper;
>       NotifyVmexitOption notify_vmexit;
>       uint32_t notify_window;
> +
> +    /* xen guest state */
> +    struct XenState xen;

Ditto.

>   };
>   
>   void kvm_memory_listener_register(KVMState *s, KVMMemoryListener *kml,
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 5ddd14467e..09c0281b8b 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -23,6 +23,14 @@
>   #include "sysemu/tcg.h"
>   #include "cpu-qom.h"
>   #include "kvm/hyperv-proto.h"
> +#include "xen-proto.h"
> +
> +#ifdef TARGET_X86_64
> +#define TARGET_LONG_BITS 64
> +#else
> +#define TARGET_LONG_BITS 32
> +#endif

How come you don't have access to the definitions from "cpu-param.h" here?

Regards,

Phil.