On Tue, Apr 14, 2020 at 05:34:29PM +0800, Philippe Mathieu-Daudé wrote:
> Hi Yan,
>
> On 4/13/20 8:37 AM, Yan Zhao wrote:
> > for vfio regions that are without write permission,
> > drop guest writes to those regions.
> >
> > Signed-off-by: Yan Zhao <yan.y.z...@intel.com>
> > Signed-off-by: Xin Zeng <xin.z...@intel.com>
> > ---
> > hw/vfio/common.c | 8 +++++++-
> > hw/vfio/trace-events | 2 +-
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > index 0b3593b3c0..fd6ee1fe3e 100644
> > --- a/hw/vfio/common.c
> > +++ b/hw/vfio/common.c
> > @@ -190,6 +190,11 @@ void vfio_region_write(void *opaque, hwaddr addr,
> > uint64_t qword;
> > } buf;
> >
>
> I'd move the trace here (trace always):
>
> trace_vfio_region_write(vbasedev->name, region->nr, addr, data,
> size);
>
> > + if (!(region->flags & VFIO_REGION_INFO_FLAG_WRITE)) {
> > + trace_vfio_region_write(vbasedev->name, region->nr,
> > + addr, data, size, true);
>
> And use qemu_log_mask(LOG_GUEST_ERROR, ...) here instead.
>
ok. will change it. Thanks!
Yan
> > + return;
> > + }
> > switch (size) {
> > case 1:
> > buf.byte = data;
> > @@ -215,7 +220,8 @@ void vfio_region_write(void *opaque, hwaddr addr,
> > addr, data, size);
> > }
> >
> > - trace_vfio_region_write(vbasedev->name, region->nr, addr, data, size);
> > + trace_vfio_region_write(vbasedev->name, region->nr, addr, data, size,
> > + false);
> >
> > /*
> > * A read or write to a BAR always signals an INTx EOI. This will
> > diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
> > index b1ef55a33f..fb9ff604e6 100644
> > --- a/hw/vfio/trace-events
> > +++ b/hw/vfio/trace-events
> > @@ -91,7 +91,7 @@ vfio_pci_nvlink2_setup_quirk_ssatgt(const char *name,
> > uint64_t tgt, uint64_t siz
> > vfio_pci_nvlink2_setup_quirk_lnkspd(const char *name, uint32_t
> > link_speed) "%s link_speed=0x%x"
> >
> > # common.c
> > -vfio_region_write(const char *name, int index, uint64_t addr, uint64_t
> > data, unsigned size) " (%s:region%d+0x%"PRIx64", 0x%"PRIx64 ", %d)"
> > +vfio_region_write(const char *name, int index, uint64_t addr, uint64_t
> > data, unsigned size, bool readonly) " (%s:region%d+0x%"PRIx64", 0x%"PRIx64
> > ", %d)" " is_readonly_region=%d."
> > vfio_region_read(char *name, int index, uint64_t addr, unsigned size,
> > uint64_t data) " (%s:region%d+0x%"PRIx64", %d) = 0x%"PRIx64
> > vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t
> > iova_end) "iommu %s @ 0x%"PRIx64" - 0x%"PRIx64
> > vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING
> > region_add 0x%"PRIx64" - 0x%"PRIx64
> >
>
