1. Patchew
  2. QEMU
  3. [PATCH v2 0/9] memory: assert and define MemoryRegionOps callbacks
  4. View patch

[PATCH v2 4/9] prep: add ppc-parity write method

P J P posted 9 patches 5 years, 7 months ago
Maintainers: Joel Stanley <joel@jms.id.au>, Peter Maydell <peter.maydell@linaro.org>, Alex Williamson <alex.williamson@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, Andrey Smirnov <andrew.smirnov@gmail.com>, "Hervé Poussineau" <hpoussin@reactos.org>
There is a newer version of this series
[PATCH v2 4/9] prep: add ppc-parity write method
Posted by P J P 5 years, 7 months ago 
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__);
+}
+
 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
Re: [PATCH v2 4/9] prep: add ppc-parity write method
Posted by Philippe Mathieu-Daudé 5 years, 7 months ago 
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,
>
Re: [PATCH v2 4/9] prep: add ppc-parity write method
Posted by Li Qiang 5 years, 7 months ago 
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
>
Re: [PATCH v2 4/9] prep: add ppc-parity write method
Posted by Philippe Mathieu-Daudé 5 years, 6 months ago 
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
>>
>
Re: [PATCH v2 4/9] prep: add ppc-parity write method
Posted by David Gibson 5 years, 7 months ago 
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,

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.