Re: [PATCH v3 3/9] vfio: add quirk device write method
On Fri, 17 Jul 2020 16:57:40 +0100 Peter Maydell wrote: > On Fri, 17 Jul 2020 at 16:54, Alex Williamson > wrote: > > > > On Thu, 16 Jul 2020 18:46:33 +0100 > > Peter Maydell wrote: > > > > > Alex (Williamson) -- as the vfio maintainer, do you have a view > > > on whether we should be logging write accesses to port 0x3c3 > > > here as guest-errors or unimplemented-QEMU-functionality? > > > > > > Guest-error seems plausible to me, anyway. > > > > I believe the intention was that writes would be dropped, so if this > > generates logging that is going to cause users to file bugs, that would > > be undesirable. Thanks, > > It will only log if the user explicitly turns on "log things the > guest does which are bugs in it, like writing to read-only > registers" (with the '-d guest_errors' option). IIRC, this quirk is based on observation more so than an actual spec, so whether a log of such an event is interpreted as a guest error or an emulation error might be up for debate. Aside from that nit, and lack of bandwidth to research how hardware handles writes, Acked-by: Alex Williamson Thanks, Alex
Re: [PATCH v3 3/9] vfio: add quirk device write method
On Fri, 17 Jul 2020 at 16:54, Alex Williamson wrote: > > On Thu, 16 Jul 2020 18:46:33 +0100 > Peter Maydell wrote: > > > Alex (Williamson) -- as the vfio maintainer, do you have a view > > on whether we should be logging write accesses to port 0x3c3 > > here as guest-errors or unimplemented-QEMU-functionality? > > > > Guest-error seems plausible to me, anyway. > > I believe the intention was that writes would be dropped, so if this > generates logging that is going to cause users to file bugs, that would > be undesirable. Thanks, It will only log if the user explicitly turns on "log things the guest does which are bugs in it, like writing to read-only registers" (with the '-d guest_errors' option). thanks -- PMM
Re: [PATCH v3 3/9] vfio: add quirk device write method
On Thu, 16 Jul 2020 18:46:33 +0100 Peter Maydell wrote: > On Tue, 30 Jun 2020 at 13:30, P J P wrote: > > > > From: Prasad J Pandit > > > > Add vfio quirk device mmio write method to avoid NULL pointer > > dereference issue. > > > > Reported-by: Lei Sun > > Reviewed-by: Li Qiang > > Signed-off-by: Prasad J Pandit > > --- > > hw/vfio/pci-quirks.c | 8 > > 1 file changed, 8 insertions(+) > > > > Update v3: Add Reviewed-by: ... > > -> https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg09406.html > > > > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c > > index d304c81148..cc6d5dbc23 100644 > > --- a/hw/vfio/pci-quirks.c > > +++ b/hw/vfio/pci-quirks.c > > @@ -14,6 +14,7 @@ > > #include "config-devices.h" > > #include "exec/memop.h" > > #include "qemu/units.h" > > +#include "qemu/log.h" > > #include "qemu/error-report.h" > > #include "qemu/main-loop.h" > > #include "qemu/module.h" > > @@ -264,8 +265,15 @@ static uint64_t vfio_ati_3c3_quirk_read(void *opaque, > > return data; > > } > > > > +static void vfio_ati_3c3_quirk_write(void *opaque, hwaddr addr, > > +uint64_t data, unsigned size) > > +{ > > +qemu_log_mask(LOG_GUEST_ERROR, "%s not implemented\n", __func__); > > +} > > + > > static const MemoryRegionOps vfio_ati_3c3_quirk = { > > .read = vfio_ati_3c3_quirk_read, > > +.write = vfio_ati_3c3_quirk_write, > > .endianness = DEVICE_LITTLE_ENDIAN, > > }; > > > Alex (Williamson) -- as the vfio maintainer, do you have a view > on whether we should be logging write accesses to port 0x3c3 > here as guest-errors or unimplemented-QEMU-functionality? > > Guest-error seems plausible to me, anyway. I believe the intention was that writes would be dropped, so if this generates logging that is going to cause users to file bugs, that would be undesirable. Thanks, Alex
Re: [PATCH v3 3/9] vfio: add quirk device write method
On Tue, 30 Jun 2020 at 13:30, P J P wrote: > > From: Prasad J Pandit > > Add vfio quirk device mmio write method to avoid NULL pointer > dereference issue. > > Reported-by: Lei Sun > Reviewed-by: Li Qiang > Signed-off-by: Prasad J Pandit > --- > hw/vfio/pci-quirks.c | 8 > 1 file changed, 8 insertions(+) > > Update v3: Add Reviewed-by: ... > -> https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg09406.html > > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c > index d304c81148..cc6d5dbc23 100644 > --- a/hw/vfio/pci-quirks.c > +++ b/hw/vfio/pci-quirks.c > @@ -14,6 +14,7 @@ > #include "config-devices.h" > #include "exec/memop.h" > #include "qemu/units.h" > +#include "qemu/log.h" > #include "qemu/error-report.h" > #include "qemu/main-loop.h" > #include "qemu/module.h" > @@ -264,8 +265,15 @@ static uint64_t vfio_ati_3c3_quirk_read(void *opaque, > return data; > } > > +static void vfio_ati_3c3_quirk_write(void *opaque, hwaddr addr, > +uint64_t data, unsigned size) > +{ > +qemu_log_mask(LOG_GUEST_ERROR, "%s not implemented\n", __func__); > +} > + > static const MemoryRegionOps vfio_ati_3c3_quirk = { > .read = vfio_ati_3c3_quirk_read, > +.write = vfio_ati_3c3_quirk_write, > .endianness = DEVICE_LITTLE_ENDIAN, > }; Alex (Williamson) -- as the vfio maintainer, do you have a view on whether we should be logging write accesses to port 0x3c3 here as guest-errors or unimplemented-QEMU-functionality? Guest-error seems plausible to me, anyway. Reviewed-by: Peter Maydell thanks -- PMM
[PATCH v3 3/9] vfio: add quirk device write method
From: Prasad J Pandit Add vfio quirk device mmio write method to avoid NULL pointer dereference issue. Reported-by: Lei Sun Reviewed-by: Li Qiang Signed-off-by: Prasad J Pandit --- hw/vfio/pci-quirks.c | 8 1 file changed, 8 insertions(+) Update v3: Add Reviewed-by: ... -> https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg09406.html diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c index d304c81148..cc6d5dbc23 100644 --- a/hw/vfio/pci-quirks.c +++ b/hw/vfio/pci-quirks.c @@ -14,6 +14,7 @@ #include "config-devices.h" #include "exec/memop.h" #include "qemu/units.h" +#include "qemu/log.h" #include "qemu/error-report.h" #include "qemu/main-loop.h" #include "qemu/module.h" @@ -264,8 +265,15 @@ static uint64_t vfio_ati_3c3_quirk_read(void *opaque, return data; } +static void vfio_ati_3c3_quirk_write(void *opaque, hwaddr addr, +uint64_t data, unsigned size) +{ +qemu_log_mask(LOG_GUEST_ERROR, "%s not implemented\n", __func__); +} + static const MemoryRegionOps vfio_ati_3c3_quirk = { .read = vfio_ati_3c3_quirk_read, +.write = vfio_ati_3c3_quirk_write, .endianness = DEVICE_LITTLE_ENDIAN, }; -- 2.26.2