| Message ID | 20221205173137.607044-11-dwmw2@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Xen HVM support under KVM | expand |
On 5/12/22 18:31, David Woodhouse wrote: > From: Joao Martins <joao.m.martins@oracle.com> > > This means handling the new exit reason for Xen but still > crashing on purpose. As we implement each of the hypercalls > we will then return the right return code. > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > [dwmw2: Add CPL to hypercall tracing, disallow hypercalls from CPL > 0] > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > target/i386/kvm/kvm.c | 5 +++++ > target/i386/trace-events | 3 +++ > target/i386/xen.c | 45 ++++++++++++++++++++++++++++++++++++++++ > target/i386/xen.h | 1 + > 4 files changed, 54 insertions(+) > > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > index 4b21d03250..6396d11f1e 100644 > --- a/target/i386/kvm/kvm.c > +++ b/target/i386/kvm/kvm.c > @@ -5468,6 +5468,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) > assert(run->msr.reason == KVM_MSR_EXIT_REASON_FILTER); > ret = kvm_handle_wrmsr(cpu, run); > break; > +#ifdef CONFIG_XEN CONFIG_XEN is set when the _host_ has Xen development files available. IIUC here you want to check if Xen HVM guest support is enabled. You might want to use a different CONFIG_XEN_xxx key, which itself depends on CONFIG_XEN. > + case KVM_EXIT_XEN: > + ret = kvm_xen_handle_exit(cpu, &run->xen); > + break; > +#endif > default: > fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason); > ret = -1; > diff --git a/target/i386/xen.h b/target/i386/xen.h > index d4903ecfa1..3537415d31 100644 > --- a/target/i386/xen.h > +++ b/target/i386/xen.h > @@ -23,5 +23,6 @@ > #define XEN_VERSION(maj, min) ((maj) << 16 | (min)) > > int kvm_xen_init(KVMState *s, uint32_t xen_version); > +int kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit); > > #endif /* QEMU_I386_XEN_H */
On Mon, 2022-12-05 at 23:11 +0100, Philippe Mathieu-Daudé wrote: > On 5/12/22 18:31, David Woodhouse wrote: > > From: Joao Martins <joao.m.martins@oracle.com> > > > > This means handling the new exit reason for Xen but still > > crashing on purpose. As we implement each of the hypercalls > > we will then return the right return code. > > > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > > [dwmw2: Add CPL to hypercall tracing, disallow hypercalls from CPL > 0] > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > > --- > > target/i386/kvm/kvm.c | 5 +++++ > > target/i386/trace-events | 3 +++ > > target/i386/xen.c | 45 ++++++++++++++++++++++++++++++++++++++++ > > target/i386/xen.h | 1 + > > 4 files changed, 54 insertions(+) > > > > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > > index 4b21d03250..6396d11f1e 100644 > > --- a/target/i386/kvm/kvm.c > > +++ b/target/i386/kvm/kvm.c > > @@ -5468,6 +5468,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) > > assert(run->msr.reason == KVM_MSR_EXIT_REASON_FILTER); > > ret = kvm_handle_wrmsr(cpu, run); > > break; > > +#ifdef CONFIG_XEN > > CONFIG_XEN is set when the _host_ has Xen development files available. > > IIUC here you want to check if Xen HVM guest support is enabled. > > You might want to use a different CONFIG_XEN_xxx key, which itself > depends on CONFIG_XEN. Yeah, I'd be interested in opinions on that one. Strictly, the only one that *needs* to be a configure option is CONFIG_XEN for the Xen libraries, which is support for actually running on Xen. Any time KVM is present, we *could* pull in the rest of the xenfv machine support unconditionally, since that's no longer dependent on true Xen. But because there's a non-trivial amount of code in the event channel and grant table stuff, *perhaps* we want to make it optional? I don't really want to call that CONFIG_KVM_XEN since as noted, it's theoretically possible to do it with TCG or other accelerators too. So we could call it CONFIG_XEN_EMULATION. I don't think we'd make that depend on CONFIG_XEN though, since none of the actual Xen libraries would be needed once everything's implemented and cleaned up. So things like the xenfv machine code would then depend on (CONFIG_XEN || CONFIG_XEN_EMULATION)... or we could make a new automatic config symbol CONFIG_XEN_MACHINE which has the same effect? Happy to do it however seems best.
+Thomas On 6/12/22 02:10, David Woodhouse wrote: > On Mon, 2022-12-05 at 23:11 +0100, Philippe Mathieu-Daudé wrote: >> On 5/12/22 18:31, David Woodhouse wrote: >>> From: Joao Martins <joao.m.martins@oracle.com> >>> >>> This means handling the new exit reason for Xen but still >>> crashing on purpose. As we implement each of the hypercalls >>> we will then return the right return code. >>> >>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >>> [dwmw2: Add CPL to hypercall tracing, disallow hypercalls from CPL > 0] >>> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> >>> --- >>> target/i386/kvm/kvm.c | 5 +++++ >>> target/i386/trace-events | 3 +++ >>> target/i386/xen.c | 45 ++++++++++++++++++++++++++++++++++++++++ >>> target/i386/xen.h | 1 + >>> 4 files changed, 54 insertions(+) >>> >>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c >>> index 4b21d03250..6396d11f1e 100644 >>> --- a/target/i386/kvm/kvm.c >>> +++ b/target/i386/kvm/kvm.c >>> @@ -5468,6 +5468,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) >>> assert(run->msr.reason == KVM_MSR_EXIT_REASON_FILTER); >>> ret = kvm_handle_wrmsr(cpu, run); >>> break; >>> +#ifdef CONFIG_XEN >> >> CONFIG_XEN is set when the _host_ has Xen development files available. >> >> IIUC here you want to check if Xen HVM guest support is enabled. >> >> You might want to use a different CONFIG_XEN_xxx key, which itself >> depends on CONFIG_XEN. > > Yeah, I'd be interested in opinions on that one. > > Strictly, the only one that *needs* to be a configure option is > CONFIG_XEN for the Xen libraries, which is support for actually running > on Xen. > > Any time KVM is present, we *could* pull in the rest of the xenfv > machine support unconditionally, since that's no longer dependent on > true Xen. > > But because there's a non-trivial amount of code in the event channel > and grant table stuff, *perhaps* we want to make it optional? I don't > really want to call that CONFIG_KVM_XEN since as noted, it's > theoretically possible to do it with TCG or other accelerators too. So > we could call it CONFIG_XEN_EMULATION. I concur CONFIG_KVM_XEN is confusing; CONFIG_XEN_EMULATION / CONFIG_XEN_EMU sounds better. Is it useful to have the CONFIG_XEN_EMU code under target/i386/ built without having the xenfv machine built in? I rather have hw/ and target/ features disentangled, so I'd use CONFIG_XEN_EMU under target/ and CONFIG_XENFV_MACHINE under hw/, eventually having CONFIG_XEN_EMU depending on CONFIG_XENFV_MACHINE and -- for now -- CONFIG_KVM. > I don't think we'd make that depend on CONFIG_XEN though, since none of > the actual Xen libraries would be needed once everything's implemented > and cleaned up. Agreed. > So things like the xenfv machine code would then depend on > (CONFIG_XEN || CONFIG_XEN_EMULATION)... or we could make a new > automatic config symbol CONFIG_XEN_MACHINE which has the same effect? So per what you just cleared, not CONFIG_XEN but CONFIG_KVM.
On Tue, 2022-12-06 at 09:16 +0100, Philippe Mathieu-Daudé wrote: > +Thomas > > On 6/12/22 02:10, David Woodhouse wrote: > > On Mon, 2022-12-05 at 23:11 +0100, Philippe Mathieu-Daudé wrote: > > > On 5/12/22 18:31, David Woodhouse wrote: > > > > +#ifdef CONFIG_XEN > > > > > > CONFIG_XEN is set when the _host_ has Xen development files available. > > > > > > IIUC here you want to check if Xen HVM guest support is enabled. > > > > > > You might want to use a different CONFIG_XEN_xxx key, which itself > > > depends on CONFIG_XEN. > > > > Yeah, I'd be interested in opinions on that one. > > > > Strictly, the only one that *needs* to be a configure option is > > CONFIG_XEN for the Xen libraries, which is support for actually running > > on Xen. > > > > Any time KVM is present, we *could* pull in the rest of the xenfv > > machine support unconditionally, since that's no longer dependent on > > true Xen. > > > > But because there's a non-trivial amount of code in the event channel > > and grant table stuff, *perhaps* we want to make it optional? I don't > > really want to call that CONFIG_KVM_XEN since as noted, it's > > theoretically possible to do it with TCG or other accelerators too. So > > we could call it CONFIG_XEN_EMULATION. > > I concur CONFIG_KVM_XEN is confusing; CONFIG_XEN_EMULATION / > CONFIG_XEN_EMU sounds better. > > Is it useful to have the CONFIG_XEN_EMU code under target/i386/ built > without having the xenfv machine built in? It isn't useful, no. > I rather have hw/ and target/ features disentangled, so I'd use > CONFIG_XEN_EMU under target/ and CONFIG_XENFV_MACHINE under hw/, > eventually having CONFIG_XEN_EMU depending on CONFIG_XENFV_MACHINE > and -- for now -- CONFIG_KVM. Hm, I was thinking of XENFV_MACHINE as the parts which are needed by *both* XEN_EMU and real Xen. I think there are arch-independent things which want to go into hw/ like event channels and grant table support; you can think of those as an IRQ chip and an IOMMU respectively. Since those are emulation-only, they want to be conditional on XEN_EMU, not XENFV_MACHINE. The core hypercall support lives in target/ and would call directly to gnttab_op/evtchn_op functions in hw/xen/ but I think that's OK. The vCPU-specific things like timers and runstate can also stay in target/. Nothing in hw/ should explicitly mention KVM; the code in target/i386/xen.c should wrap the KVM-specific implementations unless the pretence of future TCG support is really making it look awful. Does that sound reasonable? Probably close enough, and we can take an other look at it once we see how it works out. > > I don't think we'd make that depend on CONFIG_XEN though, since none of > > the actual Xen libraries would be needed once everything's implemented > > and cleaned up. > > Agreed. > > > So things like the xenfv machine code would then depend on > > (CONFIG_XEN || CONFIG_XEN_EMULATION)... or we could make a new > > automatic config symbol CONFIG_XEN_MACHINE which has the same effect? > > So per what you just cleared, not CONFIG_XEN but CONFIG_KVM. I think it looks something like this... From 0a90999e37ec48b7fbd0467c243769b9bf726401 Mon Sep 17 00:00:00 2001 From: David Woodhouse <dwmw@amazon.co.uk> Date: Tue, 6 Dec 2022 09:03:48 +0000 Subject: [PATCH] Add CONFIG_XENFV_MACHINE and CONFIG_XEN_EMU options for Xen emulation The XEN_EMU option will cover core Xen support in target/, which exists only for x86 with KVM today but could theoretically also be implemented on Arm/Aarch64 and with TCG or other accelerators. It will also cover the support for architecture-independent grant table and event channel support which will be added in hw/xen/. The XENFV_MACHINE option is for the xenfv platform support, which will now be used both by XEN_EMU and by real Xen. The XEN option remains dependent on the Xen runtime libraries, and covers support for real Xen. Some code which currently resides under CONFIG_XEN will be moving to CONFIG_XENFV_MACHINE over time. Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> --- accel/Kconfig | 2 ++ hw/Kconfig | 1 + hw/xen/Kconfig | 2 ++ meson.build | 1 + target/Kconfig | 3 +++ 5 files changed, 9 insertions(+) create mode 100644 hw/xen/Kconfig diff --git a/accel/Kconfig b/accel/Kconfig index 8bdedb7d15..87d2880cad 100644 --- a/accel/Kconfig +++ b/accel/Kconfig @@ -15,7 +15,9 @@ config TCG config KVM bool + select XEN_EMU if (I386 || X86_64) config XEN bool select FSDEV_9P if VIRTFS + select XENFV_MACHINE diff --git a/hw/Kconfig b/hw/Kconfig index 38233bbb0f..ba62ff6417 100644 --- a/hw/Kconfig +++ b/hw/Kconfig @@ -41,6 +41,7 @@ source tpm/Kconfig source usb/Kconfig source virtio/Kconfig source vfio/Kconfig +source xen/Kconfig source watchdog/Kconfig # arch Kconfig diff --git a/hw/xen/Kconfig b/hw/xen/Kconfig new file mode 100644 index 0000000000..066d74e4ff --- /dev/null +++ b/hw/xen/Kconfig @@ -0,0 +1,2 @@ +config XENFV_MACHINE + bool diff --git a/meson.build b/meson.build index 5c6b5a1c75..9348cf572c 100644 --- a/meson.build +++ b/meson.build @@ -3828,6 +3828,7 @@ if have_system if xen.found() summary_info += {'xen ctrl version': xen.version()} endif + summary_info += {'Xen emulation': config_all.has_key('CONFIG_XEN_EMU')} endif summary_info += {'TCG support': config_all.has_key('CONFIG_TCG')} if config_all.has_key('CONFIG_TCG') diff --git a/target/Kconfig b/target/Kconfig index 83da0bd293..ceb6ddbf2a 100644 --- a/target/Kconfig +++ b/target/Kconfig @@ -18,3 +18,6 @@ source sh4/Kconfig source sparc/Kconfig source tricore/Kconfig source xtensa/Kconfig + +config XEN_EMU + select XENFV_MACHINE
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > +Thomas > > On 6/12/22 02:10, David Woodhouse wrote: >> On Mon, 2022-12-05 at 23:11 +0100, Philippe Mathieu-Daudé wrote: >>> On 5/12/22 18:31, David Woodhouse wrote: >>>> From: Joao Martins <joao.m.martins@oracle.com> >>>> >>>> This means handling the new exit reason for Xen but still >>>> crashing on purpose. As we implement each of the hypercalls >>>> we will then return the right return code. >>>> >>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >>>> [dwmw2: Add CPL to hypercall tracing, disallow hypercalls from CPL > 0] >>>> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> >>>> --- >>>> target/i386/kvm/kvm.c | 5 +++++ >>>> target/i386/trace-events | 3 +++ >>>> target/i386/xen.c | 45 ++++++++++++++++++++++++++++++++++++++++ >>>> target/i386/xen.h | 1 + >>>> 4 files changed, 54 insertions(+) >>>> >>>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c >>>> index 4b21d03250..6396d11f1e 100644 >>>> --- a/target/i386/kvm/kvm.c >>>> +++ b/target/i386/kvm/kvm.c >>>> @@ -5468,6 +5468,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) >>>> assert(run->msr.reason == KVM_MSR_EXIT_REASON_FILTER); >>>> ret = kvm_handle_wrmsr(cpu, run); >>>> break; >>>> +#ifdef CONFIG_XEN >>> >>> CONFIG_XEN is set when the _host_ has Xen development files available. >>> >>> IIUC here you want to check if Xen HVM guest support is enabled. >>> >>> You might want to use a different CONFIG_XEN_xxx key, which itself >>> depends on CONFIG_XEN. >> Yeah, I'd be interested in opinions on that one. >> Strictly, the only one that *needs* to be a configure option is >> CONFIG_XEN for the Xen libraries, which is support for actually running >> on Xen. >> Any time KVM is present, we *could* pull in the rest of the xenfv >> machine support unconditionally, since that's no longer dependent on >> true Xen. >> But because there's a non-trivial amount of code in the event >> channel >> and grant table stuff, *perhaps* we want to make it optional? I don't >> really want to call that CONFIG_KVM_XEN since as noted, it's >> theoretically possible to do it with TCG or other accelerators too. So >> we could call it CONFIG_XEN_EMULATION. > > I concur CONFIG_KVM_XEN is confusing; CONFIG_XEN_EMULATION / > CONFIG_XEN_EMU sounds better. > > Is it useful to have the CONFIG_XEN_EMU code under target/i386/ built > without having the xenfv machine built in? > > I rather have hw/ and target/ features disentangled, so I'd use > CONFIG_XEN_EMU under target/ and CONFIG_XENFV_MACHINE under hw/, > eventually having CONFIG_XEN_EMU depending on CONFIG_XENFV_MACHINE > and -- for now -- CONFIG_KVM. You should also probably be aware of: Subject: [QEMU][PATCH v2 00/11] Introduce xenpv machine for arm architecture Date: Thu, 1 Dec 2022 18:59:52 -0800 Message-ID: <20221202030003.11441-1-vikram.garhwal@amd.com> which moves some of the previously i386 only Xen code into common backend code.
On 6/12/22 10:40, David Woodhouse wrote: > On Tue, 2022-12-06 at 09:16 +0100, Philippe Mathieu-Daudé wrote: >> +Thomas >> >> On 6/12/22 02:10, David Woodhouse wrote: >>> On Mon, 2022-12-05 at 23:11 +0100, Philippe Mathieu-Daudé wrote: >>>> On 5/12/22 18:31, David Woodhouse wrote: >>>>> +#ifdef CONFIG_XEN >>>> >>>> CONFIG_XEN is set when the _host_ has Xen development files available. >>>> >>>> IIUC here you want to check if Xen HVM guest support is enabled. >>>> >>>> You might want to use a different CONFIG_XEN_xxx key, which itself >>>> depends on CONFIG_XEN. >>> >>> Yeah, I'd be interested in opinions on that one. >>> >>> Strictly, the only one that *needs* to be a configure option is >>> CONFIG_XEN for the Xen libraries, which is support for actually running >>> on Xen. >>> >>> Any time KVM is present, we *could* pull in the rest of the xenfv >>> machine support unconditionally, since that's no longer dependent on >>> true Xen. >>> >>> But because there's a non-trivial amount of code in the event channel >>> and grant table stuff, *perhaps* we want to make it optional? I don't >>> really want to call that CONFIG_KVM_XEN since as noted, it's >>> theoretically possible to do it with TCG or other accelerators too. So >>> we could call it CONFIG_XEN_EMULATION. >> >> I concur CONFIG_KVM_XEN is confusing; CONFIG_XEN_EMULATION / >> CONFIG_XEN_EMU sounds better. >> >> Is it useful to have the CONFIG_XEN_EMU code under target/i386/ built >> without having the xenfv machine built in? > > It isn't useful, no. > >> I rather have hw/ and target/ features disentangled, so I'd use >> CONFIG_XEN_EMU under target/ and CONFIG_XENFV_MACHINE under hw/, >> eventually having CONFIG_XEN_EMU depending on CONFIG_XENFV_MACHINE >> and -- for now -- CONFIG_KVM. > > Hm, I was thinking of XENFV_MACHINE as the parts which are needed by > *both* XEN_EMU and real Xen. I think there are arch-independent things > which want to go into hw/ like event channels and grant table support; > you can think of those as an IRQ chip and an IOMMU respectively. Since > those are emulation-only, they want to be conditional on XEN_EMU, not > XENFV_MACHINE. > > The core hypercall support lives in target/ and would call directly to > gnttab_op/evtchn_op functions in hw/xen/ but I think that's OK. The > vCPU-specific things like timers and runstate can also stay in target/. > > Nothing in hw/ should explicitly mention KVM; the code in > target/i386/xen.c should wrap the KVM-specific implementations unless > the pretence of future TCG support is really making it look awful. > > Does that sound reasonable? Probably close enough, and we can take an > other look at it once we see how it works out. > >>> I don't think we'd make that depend on CONFIG_XEN though, since none of >>> the actual Xen libraries would be needed once everything's implemented >>> and cleaned up. >> >> Agreed. >> >>> So things like the xenfv machine code would then depend on >>> (CONFIG_XEN || CONFIG_XEN_EMULATION)... or we could make a new >>> automatic config symbol CONFIG_XEN_MACHINE which has the same effect? >> >> So per what you just cleared, not CONFIG_XEN but CONFIG_KVM. > > I think it looks something like this... > > From 0a90999e37ec48b7fbd0467c243769b9bf726401 Mon Sep 17 00:00:00 2001 > From: David Woodhouse <dwmw@amazon.co.uk> > Date: Tue, 6 Dec 2022 09:03:48 +0000 > Subject: [PATCH] Add CONFIG_XENFV_MACHINE and CONFIG_XEN_EMU options for Xen > emulation > > The XEN_EMU option will cover core Xen support in target/, which exists > only for x86 with KVM today but could theoretically also be implemented > on Arm/Aarch64 and with TCG or other accelerators. It will also cover > the support for architecture-independent grant table and event channel > support which will be added in hw/xen/. > > The XENFV_MACHINE option is for the xenfv platform support, which will > now be used both by XEN_EMU and by real Xen. > > The XEN option remains dependent on the Xen runtime libraries, and covers > support for real Xen. Some code which currently resides under CONFIG_XEN > will be moving to CONFIG_XENFV_MACHINE over time. > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > --- > accel/Kconfig | 2 ++ > hw/Kconfig | 1 + > hw/xen/Kconfig | 2 ++ > meson.build | 1 + > target/Kconfig | 3 +++ > 5 files changed, 9 insertions(+) > create mode 100644 hw/xen/Kconfig > > diff --git a/accel/Kconfig b/accel/Kconfig > index 8bdedb7d15..87d2880cad 100644 > --- a/accel/Kconfig > +++ b/accel/Kconfig > @@ -15,7 +15,9 @@ config TCG > > config KVM > bool > + select XEN_EMU if (I386 || X86_64) We certainly can build KVM without XEN_EMU... You might want to s/select/imply/ here. > config XEN > bool > select FSDEV_9P if VIRTFS > + select XENFV_MACHINE This is the XEN (host) accelerator switch... You said previously it is not related to the guest xenfv machine. > diff --git a/hw/Kconfig b/hw/Kconfig > index 38233bbb0f..ba62ff6417 100644 > --- a/hw/Kconfig > +++ b/hw/Kconfig > @@ -41,6 +41,7 @@ source tpm/Kconfig > source usb/Kconfig > source virtio/Kconfig > source vfio/Kconfig > +source xen/Kconfig > source watchdog/Kconfig > > # arch Kconfig > diff --git a/hw/xen/Kconfig b/hw/xen/Kconfig > new file mode 100644 > index 0000000000..066d74e4ff > --- /dev/null > +++ b/hw/xen/Kconfig > @@ -0,0 +1,2 @@ > +config XENFV_MACHINE > + bool select XEN_EMU > diff --git a/meson.build b/meson.build > index 5c6b5a1c75..9348cf572c 100644 > --- a/meson.build > +++ b/meson.build > @@ -3828,6 +3828,7 @@ if have_system > if xen.found() > summary_info += {'xen ctrl version': xen.version()} > endif > + summary_info += {'Xen emulation': config_all.has_key('CONFIG_XEN_EMU')} > endif > summary_info += {'TCG support': config_all.has_key('CONFIG_TCG')} > if config_all.has_key('CONFIG_TCG') > diff --git a/target/Kconfig b/target/Kconfig > index 83da0bd293..ceb6ddbf2a 100644 > --- a/target/Kconfig > +++ b/target/Kconfig > @@ -18,3 +18,6 @@ source sh4/Kconfig > source sparc/Kconfig > source tricore/Kconfig > source xtensa/Kconfig > + > +config XEN_EMU > + select XENFV_MACHINE No, a target feature should not select a hw/machine component. depends on KVM
On Tue, 2022-12-06 at 12:07 +0100, Philippe Mathieu-Daudé wrote: > On 6/12/22 10:40, David Woodhouse wrote: > > On Tue, 2022-12-06 at 09:16 +0100, Philippe Mathieu-Daudé wrote: > > > +Thomas > > > > > > On 6/12/22 02:10, David Woodhouse wrote: > > > > On Mon, 2022-12-05 at 23:11 +0100, Philippe Mathieu-Daudé > > > > wrote: > > > > > On 5/12/22 18:31, David Woodhouse wrote: > > > > > > +#ifdef CONFIG_XEN > > > > > > > > > > CONFIG_XEN is set when the _host_ has Xen development files > > > > > available. > > > > > > > > > > IIUC here you want to check if Xen HVM guest support is > > > > > enabled. > > > > > > > > > > You might want to use a different CONFIG_XEN_xxx key, which > > > > > itself > > > > > depends on CONFIG_XEN. > > > > > > > > Yeah, I'd be interested in opinions on that one. > > > > > > > > Strictly, the only one that *needs* to be a configure option is > > > > CONFIG_XEN for the Xen libraries, which is support for actually > > > > running > > > > on Xen. > > > > > > > > Any time KVM is present, we *could* pull in the rest of the > > > > xenfv > > > > machine support unconditionally, since that's no longer > > > > dependent on > > > > true Xen. > > > > > > > > But because there's a non-trivial amount of code in the event > > > > channel > > > > and grant table stuff, *perhaps* we want to make it optional? I > > > > don't > > > > really want to call that CONFIG_KVM_XEN since as noted, it's > > > > theoretically possible to do it with TCG or other accelerators > > > > too. So > > > > we could call it CONFIG_XEN_EMULATION. > > > > > > I concur CONFIG_KVM_XEN is confusing; CONFIG_XEN_EMULATION / > > > CONFIG_XEN_EMU sounds better. > > > > > > Is it useful to have the CONFIG_XEN_EMU code under target/i386/ > > > built > > > without having the xenfv machine built in? > > > > It isn't useful, no. > > > > > I rather have hw/ and target/ features disentangled, so I'd use > > > CONFIG_XEN_EMU under target/ and CONFIG_XENFV_MACHINE under hw/, > > > eventually having CONFIG_XEN_EMU depending on > > > CONFIG_XENFV_MACHINE > > > and -- for now -- CONFIG_KVM. > > > > Hm, I was thinking of XENFV_MACHINE as the parts which are needed > > by > > *both* XEN_EMU and real Xen. I think there are arch-independent > > things > > which want to go into hw/ like event channels and grant table > > support; > > you can think of those as an IRQ chip and an IOMMU respectively. > > Since > > those are emulation-only, they want to be conditional on XEN_EMU, > > not > > XENFV_MACHINE. > > > > The core hypercall support lives in target/ and would call directly > > to > > gnttab_op/evtchn_op functions in hw/xen/ but I think that's OK. The > > vCPU-specific things like timers and runstate can also stay in > > target/. > > > > Nothing in hw/ should explicitly mention KVM; the code in > > target/i386/xen.c should wrap the KVM-specific implementations > > unless > > the pretence of future TCG support is really making it look awful. > > > > Does that sound reasonable? Probably close enough, and we can take > > an > > other look at it once we see how it works out. > > > > > > I don't think we'd make that depend on CONFIG_XEN though, since > > > > none of > > > > the actual Xen libraries would be needed once everything's > > > > implemented > > > > and cleaned up. > > > > > > Agreed. > > > > > > > So things like the xenfv machine code would then depend on > > > > (CONFIG_XEN || CONFIG_XEN_EMULATION)... or we could make a new > > > > automatic config symbol CONFIG_XEN_MACHINE which has the same > > > > effect? > > > > > > So per what you just cleared, not CONFIG_XEN but CONFIG_KVM. > > > > I think it looks something like this... > > > > From 0a90999e37ec48b7fbd0467c243769b9bf726401 Mon Sep 17 00:00:00 > > 2001 > > From: David Woodhouse < > > dwmw@amazon.co.uk > > > > > Date: Tue, 6 Dec 2022 09:03:48 +0000 > > Subject: [PATCH] Add CONFIG_XENFV_MACHINE and CONFIG_XEN_EMU > > options for Xen > > emulation > > > > The XEN_EMU option will cover core Xen support in target/, which > > exists > > only for x86 with KVM today but could theoretically also be > > implemented > > on Arm/Aarch64 and with TCG or other accelerators. It will also > > cover > > the support for architecture-independent grant table and event > > channel > > support which will be added in hw/xen/. > > > > The XENFV_MACHINE option is for the xenfv platform support, which > > will > > now be used both by XEN_EMU and by real Xen. > > > > The XEN option remains dependent on the Xen runtime libraries, and > > covers > > support for real Xen. Some code which currently resides under > > CONFIG_XEN > > will be moving to CONFIG_XENFV_MACHINE over time. > > > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> > > --- > > accel/Kconfig | 2 ++ > > hw/Kconfig | 1 + > > hw/xen/Kconfig | 2 ++ > > meson.build | 1 + > > target/Kconfig | 3 +++ > > 5 files changed, 9 insertions(+) > > create mode 100644 hw/xen/Kconfig > > > > diff --git a/accel/Kconfig b/accel/Kconfig > > index 8bdedb7d15..87d2880cad 100644 > > --- a/accel/Kconfig > > +++ b/accel/Kconfig > > @@ -15,7 +15,9 @@ config TCG > > > > config KVM > > bool > > + select XEN_EMU if (I386 || X86_64) > > We certainly can build KVM without XEN_EMU... > > You might want to s/select/imply/ here. OK. > > config XEN > > bool > > select FSDEV_9P if VIRTFS > > + select XENFV_MACHINE > > This is the XEN (host) accelerator switch... You said previously > it is not related to the guest xenfv machine. The xenfv platform definition, and various other things like the Xen PCI platform device, are currently covered by CONFIG_XEN. The idea is that we'd change that to CONFIG_XENFV_MACHINE so that they can be present if *either* CONFIG_XEN or CONFIG_XEN_EMU are enabled. So (given your later comment about target features not selecting a a hw/machine component) I've made it: config XENFV_MACHINE bool default y if (XEN || XEN_EMU) How's this... From eaff99027a888aac8a5df554ef22f37dc271be66 Mon Sep 17 00:00:00 2001 From: David Woodhouse <dwmw@amazon.co.uk> Date: Tue, 6 Dec 2022 09:03:48 +0000 Subject: [PATCH] Add CONFIG_XENFV_MACHINE and CONFIG_XEN_EMU options for Xen emulation The XEN_EMU option will cover core Xen support in target/, which exists only for x86 with KVM today but could theoretically also be implemented on Arm/Aarch64 and with TCG or other accelerators. It will also cover the support for architecture-independent grant table and event channel support which will be added in hw/xen/. The XENFV_MACHINE option is for the xenfv platform support, which will now be used both by XEN_EMU and by real Xen. The XEN option remains dependent on the Xen runtime libraries, and covers support for real Xen. Some code which currently resides under CONFIG_XEN will be moving to CONFIG_XENFV_MACHINE over time. Signed-off-by: David Woodhouse <dwmw@amazon.co.uk> --- accel/Kconfig | 1 + hw/Kconfig | 1 + hw/xen/Kconfig | 3 +++ meson.build | 1 + target/Kconfig | 4 ++++ 5 files changed, 10 insertions(+) create mode 100644 hw/xen/Kconfig diff --git a/accel/Kconfig b/accel/Kconfig index 8bdedb7d15..41e089e610 100644 --- a/accel/Kconfig +++ b/accel/Kconfig @@ -15,6 +15,7 @@ config TCG config KVM bool + imply XEN_EMU if (I386 || X86_64) config XEN bool diff --git a/hw/Kconfig b/hw/Kconfig index 38233bbb0f..ba62ff6417 100644 --- a/hw/Kconfig +++ b/hw/Kconfig @@ -41,6 +41,7 @@ source tpm/Kconfig source usb/Kconfig source virtio/Kconfig source vfio/Kconfig +source xen/Kconfig source watchdog/Kconfig # arch Kconfig diff --git a/hw/xen/Kconfig b/hw/xen/Kconfig new file mode 100644 index 0000000000..755c8b1faf --- /dev/null +++ b/hw/xen/Kconfig @@ -0,0 +1,3 @@ +config XENFV_MACHINE + bool + default y if (XEN || XEN_EMU) diff --git a/meson.build b/meson.build index 5c6b5a1c75..9348cf572c 100644 --- a/meson.build +++ b/meson.build @@ -3828,6 +3828,7 @@ if have_system if xen.found() summary_info += {'xen ctrl version': xen.version()} endif + summary_info += {'Xen emulation': config_all.has_key('CONFIG_XEN_EMU')} endif summary_info += {'TCG support': config_all.has_key('CONFIG_TCG')} if config_all.has_key('CONFIG_TCG') diff --git a/target/Kconfig b/target/Kconfig index 83da0bd293..e19c9d77b5 100644 --- a/target/Kconfig +++ b/target/Kconfig @@ -18,3 +18,7 @@ source sh4/Kconfig source sparc/Kconfig source tricore/Kconfig source xtensa/Kconfig + +config XEN_EMU + bool + depends on KVM && (I386 || X86_64)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 4b21d03250..6396d11f1e 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -5468,6 +5468,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run) assert(run->msr.reason == KVM_MSR_EXIT_REASON_FILTER); ret = kvm_handle_wrmsr(cpu, run); break; +#ifdef CONFIG_XEN + case KVM_EXIT_XEN: + ret = kvm_xen_handle_exit(cpu, &run->xen); + break; +#endif default: fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason); ret = -1; diff --git a/target/i386/trace-events b/target/i386/trace-events index 2cd8726eeb..3fb9ee3add 100644 --- a/target/i386/trace-events +++ b/target/i386/trace-events @@ -11,3 +11,6 @@ kvm_sev_launch_measurement(const char *value) "data %s" kvm_sev_launch_finish(void) "" kvm_sev_launch_secret(uint64_t hpa, uint64_t hva, uint64_t secret, int len) "hpa 0x%" PRIx64 " hva 0x%" PRIx64 " data 0x%" PRIx64 " len %d" kvm_sev_attestation_report(const char *mnonce, const char *data) "mnonce %s data %s" + +# target/i386/xen.c +kvm_xen_hypercall(int cpu, uint8_t cpl, uint64_t input, uint64_t a0, uint64_t a1, uint64_t a2, uint64_t ret) "xen_hypercall: cpu %d cpl %d input %" PRIu64 " a0 0x%" PRIx64 " a1 0x%" PRIx64 " a2 0x%" PRIx64" ret 0x%" PRIu64 diff --git a/target/i386/xen.c b/target/i386/xen.c index bc183dce4e..d7e942289c 100644 --- a/target/i386/xen.c +++ b/target/i386/xen.c @@ -12,6 +12,17 @@ #include "qemu/osdep.h" #include "kvm/kvm_i386.h" #include "xen.h" +#include "trace.h" + +/* + * Unhandled hypercalls error: + * + * -1 crash and dump registers + * 0 no abort and guest handles -ENOSYS (default) + */ +#ifndef HCALL_ERR +#define HCALL_ERR 0 +#endif int kvm_xen_init(KVMState *s, uint32_t xen_version) { @@ -47,3 +58,37 @@ int kvm_xen_init(KVMState *s, uint32_t xen_version) return 0; } + +static int __kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) +{ + uint16_t code = exit->u.hcall.input; + + if (exit->u.hcall.cpl > 0) { + exit->u.hcall.result = -EPERM; + return HCALL_ERR; + } + + switch (code) { + default: + exit->u.hcall.result = -ENOSYS; + return HCALL_ERR; + } +} + +int kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit) +{ + int ret = HCALL_ERR; + + switch (exit->type) { + case KVM_EXIT_XEN_HCALL: { + ret = __kvm_xen_handle_exit(cpu, exit); + trace_kvm_xen_hypercall(CPU(cpu)->cpu_index, exit->u.hcall.cpl, + exit->u.hcall.input, exit->u.hcall.params[0], + exit->u.hcall.params[1], exit->u.hcall.params[2], + exit->u.hcall.result); + return ret; + } + default: + return ret; + } +} diff --git a/target/i386/xen.h b/target/i386/xen.h index d4903ecfa1..3537415d31 100644 --- a/target/i386/xen.h +++ b/target/i386/xen.h @@ -23,5 +23,6 @@ #define XEN_VERSION(maj, min) ((maj) << 16 | (min)) int kvm_xen_init(KVMState *s, uint32_t xen_version); +int kvm_xen_handle_exit(X86CPU *cpu, struct kvm_xen_exit *exit); #endif /* QEMU_I386_XEN_H */