Message ID | 20200624185523.762240-5-ppandit@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | memory: assert and define MemoryRegionOps callbacks | expand |
On Thu, Jun 25, 2020 at 12:25:18AM +0530, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > Add ppc-parity mmio write method to avoid NULL pointer dereference > issue. > > Reported-by: Lei Sun <slei.casper@gmail.com> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> Acked-by: David Gibson <david@gibson.dropbear.id.au> > --- > hw/ppc/prep_systemio.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > Update v2: use LOG_GUEST_ERROR > -> https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg04975.html > > diff --git a/hw/ppc/prep_systemio.c b/hw/ppc/prep_systemio.c > index bbc51b6e9a..03a2f8af6e 100644 > --- a/hw/ppc/prep_systemio.c > +++ b/hw/ppc/prep_systemio.c > @@ -23,6 +23,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/log.h" > #include "hw/irq.h" > #include "hw/isa/isa.h" > #include "hw/qdev-properties.h" > @@ -235,8 +236,15 @@ static uint64_t ppc_parity_error_readl(void *opaque, hwaddr addr, > return val; > } > > +static void ppc_parity_error_writel(void *opaque, hwaddr addr, > + uint64_t data, unsigned size) > +{ > + qemu_log_mask(LOG_GUEST_ERROR, "%s not implemented\n", __func__); > +} > + > static const MemoryRegionOps ppc_parity_error_ops = { > .read = ppc_parity_error_readl, > + .write = ppc_parity_error_writel, > .valid = { > .min_access_size = 4, > .max_access_size = 4,
On 6/24/20 8:55 PM, P J P wrote: > From: Prasad J Pandit <pjp@fedoraproject.org> > > Add ppc-parity mmio write method to avoid NULL pointer dereference > issue. > > Reported-by: Lei Sun <slei.casper@gmail.com> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/ppc/prep_systemio.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > Update v2: use LOG_GUEST_ERROR > -> https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg04975.html > > diff --git a/hw/ppc/prep_systemio.c b/hw/ppc/prep_systemio.c > index bbc51b6e9a..03a2f8af6e 100644 > --- a/hw/ppc/prep_systemio.c > +++ b/hw/ppc/prep_systemio.c > @@ -23,6 +23,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/log.h" > #include "hw/irq.h" > #include "hw/isa/isa.h" > #include "hw/qdev-properties.h" > @@ -235,8 +236,15 @@ static uint64_t ppc_parity_error_readl(void *opaque, hwaddr addr, > return val; > } > > +static void ppc_parity_error_writel(void *opaque, hwaddr addr, > + uint64_t data, unsigned size) > +{ > + qemu_log_mask(LOG_GUEST_ERROR, "%s not implemented\n", __func__); What I meant was an error message about illegal/invalid write access. > +} > + > static const MemoryRegionOps ppc_parity_error_ops = { > .read = ppc_parity_error_readl, > + .write = ppc_parity_error_writel, > .valid = { > .min_access_size = 4, > .max_access_size = 4, >
P J P <ppandit@redhat.com> 于2020年6月25日周四 上午3:00写道: > > From: Prasad J Pandit <pjp@fedoraproject.org> > > Add ppc-parity mmio write method to avoid NULL pointer dereference > issue. > > Reported-by: Lei Sun <slei.casper@gmail.com> > Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> > --- > hw/ppc/prep_systemio.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > Update v2: use LOG_GUEST_ERROR > -> https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg04975.html > > diff --git a/hw/ppc/prep_systemio.c b/hw/ppc/prep_systemio.c > index bbc51b6e9a..03a2f8af6e 100644 > --- a/hw/ppc/prep_systemio.c > +++ b/hw/ppc/prep_systemio.c > @@ -23,6 +23,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/log.h" > #include "hw/irq.h" > #include "hw/isa/isa.h" > #include "hw/qdev-properties.h" > @@ -235,8 +236,15 @@ static uint64_t ppc_parity_error_readl(void *opaque, hwaddr addr, > return val; > } > > +static void ppc_parity_error_writel(void *opaque, hwaddr addr, > + uint64_t data, unsigned size) > +{ > + qemu_log_mask(LOG_GUEST_ERROR, "%s not implemented\n", __func__); > +} I'm not familiar with this platform-related stuff. I think a 'qemu_log_mask(LOG_UNIMP, xxx)' is ok. Thanks, Li Qiang > + > static const MemoryRegionOps ppc_parity_error_ops = { > .read = ppc_parity_error_readl, > + .write = ppc_parity_error_writel, > .valid = { > .min_access_size = 4, > .max_access_size = 4, > -- > 2.26.2 >
On 6/29/20 1:22 PM, Li Qiang wrote: > P J P <ppandit@redhat.com> 于2020年6月25日周四 上午3:00写道: >> >> From: Prasad J Pandit <pjp@fedoraproject.org> >> >> Add ppc-parity mmio write method to avoid NULL pointer dereference >> issue. >> >> Reported-by: Lei Sun <slei.casper@gmail.com> >> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org> >> --- >> hw/ppc/prep_systemio.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> Update v2: use LOG_GUEST_ERROR >> -> https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg04975.html >> >> diff --git a/hw/ppc/prep_systemio.c b/hw/ppc/prep_systemio.c >> index bbc51b6e9a..03a2f8af6e 100644 >> --- a/hw/ppc/prep_systemio.c >> +++ b/hw/ppc/prep_systemio.c >> @@ -23,6 +23,7 @@ >> */ >> >> #include "qemu/osdep.h" >> +#include "qemu/log.h" >> #include "hw/irq.h" >> #include "hw/isa/isa.h" >> #include "hw/qdev-properties.h" >> @@ -235,8 +236,15 @@ static uint64_t ppc_parity_error_readl(void *opaque, hwaddr addr, >> return val; >> } >> >> +static void ppc_parity_error_writel(void *opaque, hwaddr addr, >> + uint64_t data, unsigned size) >> +{ >> + qemu_log_mask(LOG_GUEST_ERROR, "%s not implemented\n", __func__); >> +} > > I'm not familiar with this platform-related stuff. I think a > 'qemu_log_mask(LOG_UNIMP, xxx)' is ok. No. The message is indeed confusing, as Prasad mixed GuestError VS UnimplementedFeature. Guest should not write the the error parity registers (I suppose they are read-only). If it does, the hardware won't behave incorrectly, it will just ignore the invalid accesses. We want to report an incorrect guest behavior (why is the guest code trying to do that?). Also, we will never implement this. We usually use something like: qemu_log_mask(LOG_GUEST_ERROR, "%s: Illegal write to read-only register " "(size %u, offset 0x%"HWADDR_PRIx"," " value 0x04%"PRIx64")\n", __func__, size, addr, data); > > Thanks, > Li Qiang > >> + >> static const MemoryRegionOps ppc_parity_error_ops = { >> .read = ppc_parity_error_readl, >> + .write = ppc_parity_error_writel, >> .valid = { >> .min_access_size = 4, >> .max_access_size = 4, >> -- >> 2.26.2 >> >
diff --git a/hw/ppc/prep_systemio.c b/hw/ppc/prep_systemio.c index bbc51b6e9a..03a2f8af6e 100644 --- a/hw/ppc/prep_systemio.c +++ b/hw/ppc/prep_systemio.c @@ -23,6 +23,7 @@ */ #include "qemu/osdep.h" +#include "qemu/log.h" #include "hw/irq.h" #include "hw/isa/isa.h" #include "hw/qdev-properties.h" @@ -235,8 +236,15 @@ static uint64_t ppc_parity_error_readl(void *opaque, hwaddr addr, return val; } +static void ppc_parity_error_writel(void *opaque, hwaddr addr, + uint64_t data, unsigned size) +{ + qemu_log_mask(LOG_GUEST_ERROR, "%s not implemented\n", __func__); +} + static const MemoryRegionOps ppc_parity_error_ops = { .read = ppc_parity_error_readl, + .write = ppc_parity_error_writel, .valid = { .min_access_size = 4, .max_access_size = 4,