| Message ID | 1603352576-21671-1-git-send-email-mihai.carabas@oracle.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add support for pvpanic mmio device | expand |
On Thu, 22 Oct 2020 at 09:25, Mihai Carabas <mihai.carabas@oracle.com> wrote: > The patchset was assembled from chuncks from some old patches from 2018 [1] > which were left unmerged and some additions from me. Surprisingly their Linux > kernel counterpart were merged (so the pvpanic driver from the kernel supports > mmio). > > I have seen the discussions about moving the pvpanic to PCI [1]. Those patches > were sent but nothing happened. Also they are not trivial and require major > modifications at the driver level also. Given the fact that we already have > mmio driver support for pvpanic in the Linux kernel, I have sent these patches > to ask again the maintainers if this can be merged. I'm afraid the answer is still the same. You need to provide a convincing argument for why this needs to be an MMIO device rather than a PCI device. I really don't want to add MMIO devices to the virt board if I can avoid it, because they're all extra code and potential extra security boundary attack surface. PCI devices are guest probeable and user-pluggable so they're almost always nicer to use than MMIO. thanks -- PMM
On 10/22/2020 1:17 PM, Peter Maydell wrote: > On Thu, 22 Oct 2020 at 09:25, Mihai Carabas <mihai.carabas@oracle.com> wrote: >> The patchset was assembled from chuncks from some old patches from 2018 [1] >> which were left unmerged and some additions from me. Surprisingly their Linux >> kernel counterpart were merged (so the pvpanic driver from the kernel supports >> mmio). >> >> I have seen the discussions about moving the pvpanic to PCI [1]. Those patches >> were sent but nothing happened. Also they are not trivial and require major >> modifications at the driver level also. Given the fact that we already have >> mmio driver support for pvpanic in the Linux kernel, I have sent these patches >> to ask again the maintainers if this can be merged. > > I'm afraid the answer is still the same. You need to provide > a convincing argument for why this needs to be an MMIO > device rather than a PCI device. I really don't want to > add MMIO devices to the virt board if I can avoid it, > because they're all extra code and potential extra > security boundary attack surface. PCI devices are guest > probeable and user-pluggable so they're almost always > nicer to use than MMIO. > Thank you for your input. The reason why pvpanic should be MMIO is that is a special device which is not used commonly by the user (aka VM), it is not need to be hot-plugable and it does not have a hardware correspondent to be a PCI device. Another reason is that MMIO support was accepted in the kernel driver and it is pretty useless there without a device. I know it seems that I want to get this on the short-path, but at this point having a kernel driver in the upstream and no device to test it against it is pretty weird. Thank you, Mihai
On Mon, 26 Oct 2020 at 13:51, Mihai Carabas <mihai.carabas@oracle.com> wrote: > On 10/22/2020 1:17 PM, Peter Maydell wrote: > > I'm afraid the answer is still the same. You need to provide > > a convincing argument for why this needs to be an MMIO > > device rather than a PCI device. I really don't want to > > add MMIO devices to the virt board if I can avoid it, > > because they're all extra code and potential extra > > security boundary attack surface. PCI devices are guest > > probeable and user-pluggable so they're almost always > > nicer to use than MMIO. > > > > Thank you for your input. > > The reason why pvpanic should be MMIO is that is a special device which > is not used commonly by the user (aka VM), it is not need to be > hot-plugable and it does not have a hardware correspondent to be a PCI > device. "Not used commonly by the user" is a good argument for "just use PCI" -- users who want the functionality can add it to their QEMU command line or VM config, and the bulk of users who don't don't have to worry about it. PCI devices don't have to support hotplug; I agree you don't need hotplug for this use case. Using a PCI device would match up with the way the x86 pvpanic device is an ISA bus device. > Another reason is that MMIO support was accepted in the kernel > driver and it is pretty useless there without a device. > I know it seems that I want to get this on the short-path, but at this > point having a kernel driver in the upstream and no device to test it > against it is pretty weird. That's the kernel folks' problem to deal with, because they accepted the kernel driver without having confirmed that QEMU was going to implement it. If I were them I'd deprecate and delete that driver code. (They should certainly fix the bit of the documentation that claims that QEMU implements it.) thanks -- PMM
The patchset was assembled from chuncks from some old patches from 2018 [1] which were left unmerged and some additions from me. Surprisingly their Linux kernel counterpart were merged (so the pvpanic driver from the kernel supports mmio). I have seen the discussions about moving the pvpanic to PCI [1]. Those patches were sent but nothing happened. Also they are not trivial and require major modifications at the driver level also. Given the fact that we already have mmio driver support for pvpanic in the Linux kernel, I have sent these patches to ask again the maintainers if this can be merged. How to test this: /usr/bin/qemu-system-aarch64 \ -machine virt,gic-version=3,pvpanic=on There is a new property for machine virt called pvpanic which is by default turned off. [1] http://patchwork.ozlabs.org/project/qemu-devel/cover/1543865209-50994-1-git-send-email-peng.hao2@zte.com.cn/ Mihai Carabas (1): pvpanic: break dependency on ISA_BUS Peng Hao (5): hw/misc/pvpanic: Add the MMIO interface hw/arm/virt: Use the pvpanic device hw/arm/virt: add pvpanic device in virt acpi table hw/arm/virt: add configure interface for pvpanic-mmio pvpanic : update pvpanic document Philippe Mathieu-Daudé (2): hw/misc/pvpanic: Build the pvpanic device for any machine hw/misc/pvpanic: Cosmetic renaming docs/specs/pvpanic.txt | 12 ++++++- hw/arm/Kconfig | 1 + hw/arm/virt-acpi-build.c | 17 ++++++++++ hw/arm/virt.c | 47 +++++++++++++++++++++++++++ hw/misc/Kconfig | 2 +- hw/misc/meson.build | 2 +- hw/misc/pvpanic.c | 83 ++++++++++++++++++++++++++++++++++++++--------- include/hw/arm/virt.h | 2 ++ include/hw/misc/pvpanic.h | 5 +-- 9 files changed, 150 insertions(+), 21 deletions(-)