| Message ID | 20221209095612.689243-3-dwmw2@infradead.org |
|---|---|
| State | New |
| Headers | show |
| Series | Xen HVM support under KVM | expand |
On 09/12/2022 09:55, David Woodhouse wrote: > From: David Woodhouse <dwmw@amazon.co.uk> > > 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 > Reviewed-by: Paul Durrant <paul@xen.org>
On 12/9/22 10:55, David Woodhouse wrote: > config KVM > bool > + imply XEN_EMU if (I386 || X86_64) No need for the "imply", just make it "default y" below and it will have the same effect. > > 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) Please place it in hw/xen/Kconfig. Paolo
On Mon, 2022-12-12 at 18:07 +0100, Paolo Bonzini wrote: > On 12/9/22 10:55, David Woodhouse wrote: > > config KVM > > bool > > + imply XEN_EMU if (I386 || X86_64) > > No need for the "imply", just make it "default y" below and it will have > the same effect. > > > > > 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) > > Please place it in hw/xen/Kconfig. Perhaps I misunderstand, but I'm not sure that is consistent with what Philippe was asking for in https://lore.kernel.org/qemu-devel/d203e13d-e2f9-5816-030d-c1449bde364d@linaro.org/ specifically: >> 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. The idea there seems to be that XEN_EMU is a *target* feature since it covers the support in target/i386/kvm. But yes, it *also* covers the devices I'm adding to hw/i386/kvm. Do I want a *separate* config symbol for that? Or just make those depend on XENFV_MACHINE && XEN_EMU ? I'll move XEN_EMU to hw/i386/Kconfig for now, thereby doing what *neither* of you said (I don't think hw/xen/Kconfig is the best choice when the *code* it enables is under hw/i386/kvm?)
Il lun 12 dic 2022, 23:23 David Woodhouse <dwmw2@infradead.org> ha scritto: > On Mon, 2022-12-12 at 18:07 +0100, Paolo Bonzini wrote: > > On 12/9/22 10:55, David Woodhouse wrote: > > > config KVM > > > bool > > > + imply XEN_EMU if (I386 || X86_64) > > > > No need for the "imply", just make it "default y" below and it will have > > the same effect. > > > > > > > > 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) > > > > Please place it in hw/xen/Kconfig. > > Perhaps I misunderstand, but I'm not sure that is consistent with what > Philippe was asking for in > > https://lore.kernel.org/qemu-devel/d203e13d-e2f9-5816-030d-c1449bde364d@linaro.org/ > specifically: > > >> 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. > However the dependency of the xenfv machine is misguided. In principle there is no reason to depend on KVM as opposed to TCG, too, which is why I didn't suggest hw/i386/kvm for either the devices or the Kconfig file. The idea there seems to be that XEN_EMU is a *target* feature since it > covers the support in target/i386/kvm. > > But yes, it *also* covers the devices I'm adding to hw/i386/kvm. Do I > want a *separate* config symbol for that? Or just make those depend on > XENFV_MACHINE && XEN_EMU ? > > I'll move XEN_EMU to hw/i386/Kconfig for now, thereby doing what > *neither* of you said (I don't think hw/xen/Kconfig is the best choice > when the *code* it enables is under hw/i386/kvm?) > Yeah there is no especially better match. I guess hw/i386 is fine, since there will be code in mc->kvm_type. It would be either there or in target/i386/Kconfig, but not target/Kconfig. Paolo >
On Tue, 2022-12-13 at 01:39 +0100, Paolo Bonzini wrote: > > > Il lun 12 dic 2022, 23:23 David Woodhouse <dwmw2@infradead.org> ha > scritto: > > On Mon, 2022-12-12 at 18:07 +0100, Paolo Bonzini wrote: > > > On 12/9/22 10:55, David Woodhouse wrote: > > > > config KVM > > > > bool > > > > + imply XEN_EMU if (I386 || X86_64) > > > > > > No need for the "imply", just make it "default y" below and it > > will have > > > the same effect. > > > > > > > > > > > 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) > > > > > > Please place it in hw/xen/Kconfig. > > > > Perhaps I misunderstand, but I'm not sure that is consistent with > > what > > Philippe was asking for in > > https://lore.kernel.org/qemu-devel/d203e13d-e2f9-5816-030d-c1449bde364d@linaro.org/ > > specifically: > > > > >> 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. > > > However the dependency of the xenfv machine is misguided. In > principle there is no reason to depend on KVM as opposed to TCG, too, > which is why I didn't suggest hw/i386/kvm for either the devices or > the Kconfig file. > That was my initial thought too. But those devices are there primarily as hooks to save/restore state, and that means they want to actually program that state back into KVM on restore; they *have* ended up using KVM directly, which is why I moved them into hw/i386/kvm. Contriving some pretence at a "generic" way for the target to set those features seemed a bit like overengineering. Supporting TCG (at least if we want to run on non-x86 hosts) isn't just a case of reimplementing the bits that are already done in-kernel, either. The structure layouts may differ too (it's bad enough between i686 and x86_64 with the alignment of uint64_t). So I just don't see TCG support happening any time soon. > > The idea there seems to be that XEN_EMU is a *target* feature since > > it covers the support in target/i386/kvm. > > > > But yes, it *also* covers the devices I'm adding to hw/i386/kvm. Do > > I want a *separate* config symbol for that? Or just make those > > depend on XENFV_MACHINE && XEN_EMU ? > > > > I'll move XEN_EMU to hw/i386/Kconfig for now, thereby doing what > > *neither* of you said (I don't think hw/xen/Kconfig is the best > > choice when the *code* it enables is under hw/i386/kvm?) > > > Yeah there is no especially better match. I guess hw/i386 is fine, > since there will be code in mc->kvm_type. It would be either there or > in target/i386/Kconfig, but not target/Kconfig. I put pc_machine_kvm_type into hw/i386/pc.c and made DEFINE_PC_MACHINE add it unconditionally. Then it registers those devices if xen_mode==XEN_EMULATE. Which is *almost* pretending to be generic, apart from the hook being KVM-specific. There was *also* a call to xen_emulated_machine_init() added to pc_init1() by the 'pc_piix: handle XEN_EMULATE backend init' patch. I've dropped that for now; once we are ready to hook up the xenbus and PV drivers, that seems like it can go into mc->kvm_type too. Or maybe I should have kept that, and initialised the overlay and evtchn devices from xen_emulated_machine_init() instead of mc->kvm_type() ?
Il mar 13 dic 2022, 01:59 David Woodhouse <dwmw2@infradead.org> ha scritto: > There was *also* a call to xen_emulated_machine_init() added to > pc_init1() by the 'pc_piix: handle XEN_EMULATE backend init' patch. > I've dropped that for now; once we are ready to hook up the xenbus and > PV drivers, that seems like it can go into mc->kvm_type too. Or maybe I > should have kept that, and initialised the overlay and evtchn devices > from xen_emulated_machine_init() instead of mc->kvm_type() ? > I think that would be too early. mc->kvm_type, which really should be called mc->accel_created, is the first place where you can look at xen_mode. Paolo Paolo >
On Tue, 2022-12-13 at 23:32 +0100, Paolo Bonzini wrote: > Il mar 13 dic 2022, 01:59 David Woodhouse <dwmw2@infradead.org> ha scritto: > > There was *also* a call to xen_emulated_machine_init() added to > > pc_init1() by the 'pc_piix: handle XEN_EMULATE backend init' patch. > > I've dropped that for now; once we are ready to hook up the xenbus and > > PV drivers, that seems like it can go into mc->kvm_type too. Or maybe I > > should have kept that, and initialised the overlay and evtchn devices > > from xen_emulated_machine_init() instead of mc->kvm_type() ? > > > I think that would be too early. mc->kvm_type, which really should be > called mc->accel_created, is the first place where you can look at > xen_mode. Ack. So now I instantiate the overlay and evtchn devices from pc_machine_kvm_type() when needed. Thanks. I've followed the HPET example and wired up a full set of NUM_GSI_PINS outbound IRQs, each connected to the corresponding system GSI, and fire the one I want to according to the guest's CALLBACK_PARAM setup. But *connecting* those had to be done later than kvm_type(), so I've done it from pc_basic_device_init() right alongside the HPET one. It couldn't work out how to qdev_find_recursive() the evtchn device, since it lacks an ID. So I just called a function in xen_evtchn.c which has access to that singleton you already told me not to worry about. The IRQs still concern me a little. I could have sworn that QEMU coped with shared level IRQs, but I can't see how. It looks like each 'output' connected to a qemu_irq input gets to waggle it up and down independently, overriding the others? Maybe it was Xen itself that keeps a *count* of how many outputs have asserted a given input, and each output is responsible for ensuring it only ever contributes a count of 0 or 1 to that total? Part of me wants to go off at a tangent and fix that in QEMU, allowing shared level IRQs as well as the EOI-driven resampling. But December only lasts so long before the US wakes up and remembers that you exist in January, and I *really* want to get the Xen support to the point where I can run XTF tests under it, at the very least...
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)