[PATCH 1/9] hw/riscv/sifive_e: Fix inheritance of SiFiveEState

Bernhard Beschow posted 9 patches 3 years, 4 months ago
Maintainers: Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <r.bolshakov@yadro.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eric Auger <eric.auger@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Laurent Vivier <laurent@vivier.eu>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Yanan Wang <wangyanan55@huawei.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Alistair Francis <alistair@alistair23.me>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Helge Deller <deller@gmx.de>, "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>, "Michael S. Tsirkin" <mst@redhat.com>, Peter Xu <peterx@redhat.com>, Jason Wang <jasowang@redhat.com>, Sergio Lopez <slp@redhat.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul@xen.org>, John Snow <jsnow@redhat.com>, "Cédric Le Goater" <clg@kaod.org>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>, Havard Skinnemoen <hskinnemoen@google.com>, Tyrone Ting <kfting@nuvoton.com>, Stafford Horne <shorne@gmail.com>, Elena Ufimtseva <elena.ufimtseva@oracle.com>, Jagannathan Raman <jag.raman@oracle.com>, John G Johnson <john.g.johnson@oracle.com>, Palmer Dabbelt <palmer@dabbelt.com>, Bin Meng <bin.meng@windriver.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, David Hildenbrand <david@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Thomas Huth <thuth@redhat.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, Magnus Damm <magnus.damm@gmail.com>, Artyom Tarasenko <atar4qemu@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>, Alex Williamson <alex.williamson@redhat.com>, Tony Krowiak <akrowiak@linux.ibm.com>, Jason Herne <jjherne@linux.ibm.com>, Alexey Kardashevskiy <aik@ozlabs.ru>, Markus Armbruster <armbru@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Alexander Graf <agraf@csgraf.de>, Michael Rolnik <mrolnik@gmail.com>, Wenchao Wang <wenchao.wang@intel.com>, Kamil Rytarowski <kamil@netbsd.org>, Reinoud Zandijk <reinoud@netbsd.org>, Marcelo Tosatti <mtosatti@redhat.com>, Sunil Muthuswamy <sunilmut@microsoft.com>, Song Gao <gaosong@loongson.cn>, Xiaojuan Yang <yangxiaojuan@loongson.cn>, Max Filippov <jcmvbkbc@gmail.com>, Alexander Bulekov <alxndr@bu.edu>, Bandan Das <bsd@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Darren Kenny <darren.kenny@oracle.com>, Qiuhao Li <Qiuhao.Li@outlook.com>
[PATCH 1/9] hw/riscv/sifive_e: Fix inheritance of SiFiveEState
Posted by Bernhard Beschow 3 years, 4 months ago
SiFiveEState inherits from SysBusDevice while it's TypeInfo claims it to
inherit from TYPE_MACHINE. This is an inconsistency which can cause
undefined behavior such as memory corruption.

Change SiFiveEState to inherit from MachineState since it is registered
as a machine.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 include/hw/riscv/sifive_e.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
index 83604da805..d738745925 100644
--- a/include/hw/riscv/sifive_e.h
+++ b/include/hw/riscv/sifive_e.h
@@ -22,6 +22,7 @@
 #include "hw/riscv/riscv_hart.h"
 #include "hw/riscv/sifive_cpu.h"
 #include "hw/gpio/sifive_gpio.h"
+#include "hw/boards.h"
 
 #define TYPE_RISCV_E_SOC "riscv.sifive.e.soc"
 #define RISCV_E_SOC(obj) \
@@ -41,7 +42,7 @@ typedef struct SiFiveESoCState {
 
 typedef struct SiFiveEState {
     /*< private >*/
-    SysBusDevice parent_obj;
+    MachineState parent_obj;
 
     /*< public >*/
     SiFiveESoCState soc;
-- 
2.37.3
Re: [PATCH 1/9] hw/riscv/sifive_e: Fix inheritance of SiFiveEState
Posted by Philippe Mathieu-Daudé 3 years, 4 months ago
On 20/9/22 01:17, Bernhard Beschow wrote:
> SiFiveEState inherits from SysBusDevice while it's TypeInfo claims it to
> inherit from TYPE_MACHINE. This is an inconsistency which can cause
> undefined behavior such as memory corruption.
> 
> Change SiFiveEState to inherit from MachineState since it is registered
> as a machine.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   include/hw/riscv/sifive_e.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
> index 83604da805..d738745925 100644
> --- a/include/hw/riscv/sifive_e.h
> +++ b/include/hw/riscv/sifive_e.h
> @@ -22,6 +22,7 @@
>   #include "hw/riscv/riscv_hart.h"
>   #include "hw/riscv/sifive_cpu.h"
>   #include "hw/gpio/sifive_gpio.h"
> +#include "hw/boards.h"
>   
>   #define TYPE_RISCV_E_SOC "riscv.sifive.e.soc"
>   #define RISCV_E_SOC(obj) \
> @@ -41,7 +42,7 @@ typedef struct SiFiveESoCState {
>   
>   typedef struct SiFiveEState {
>       /*< private >*/
> -    SysBusDevice parent_obj;
> +    MachineState parent_obj;

Ouch.

Fixes: 0869490b1c ("riscv: sifive_e: Manually define the machine")

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 1/9] hw/riscv/sifive_e: Fix inheritance of SiFiveEState
Posted by Alistair Francis 3 years, 4 months ago
On Tue, Sep 20, 2022 at 9:18 AM Bernhard Beschow <shentey@gmail.com> wrote:
>
> SiFiveEState inherits from SysBusDevice while it's TypeInfo claims it to
> inherit from TYPE_MACHINE. This is an inconsistency which can cause
> undefined behavior such as memory corruption.
>
> Change SiFiveEState to inherit from MachineState since it is registered
> as a machine.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/hw/riscv/sifive_e.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
> index 83604da805..d738745925 100644
> --- a/include/hw/riscv/sifive_e.h
> +++ b/include/hw/riscv/sifive_e.h
> @@ -22,6 +22,7 @@
>  #include "hw/riscv/riscv_hart.h"
>  #include "hw/riscv/sifive_cpu.h"
>  #include "hw/gpio/sifive_gpio.h"
> +#include "hw/boards.h"
>
>  #define TYPE_RISCV_E_SOC "riscv.sifive.e.soc"
>  #define RISCV_E_SOC(obj) \
> @@ -41,7 +42,7 @@ typedef struct SiFiveESoCState {
>
>  typedef struct SiFiveEState {
>      /*< private >*/
> -    SysBusDevice parent_obj;
> +    MachineState parent_obj;
>
>      /*< public >*/
>      SiFiveESoCState soc;
> --
> 2.37.3
>
>
Re: [PATCH 1/9] hw/riscv/sifive_e: Fix inheritance of SiFiveEState
Posted by Markus Armbruster 3 years, 4 months ago
Alistair Francis <alistair23@gmail.com> writes:

> On Tue, Sep 20, 2022 at 9:18 AM Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> SiFiveEState inherits from SysBusDevice while it's TypeInfo claims it to
>> inherit from TYPE_MACHINE. This is an inconsistency which can cause
>> undefined behavior such as memory corruption.
>>
>> Change SiFiveEState to inherit from MachineState since it is registered
>> as a machine.
>>
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

To the SiFive maintainers: since this is a bug fix, let's merge it right
away.
Re: [PATCH 1/9] hw/riscv/sifive_e: Fix inheritance of SiFiveEState
Posted by Bernhard Beschow 3 years, 4 months ago
Am 20. September 2022 11:36:47 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>Alistair Francis <alistair23@gmail.com> writes:
>
>> On Tue, Sep 20, 2022 at 9:18 AM Bernhard Beschow <shentey@gmail.com> wrote:
>>>
>>> SiFiveEState inherits from SysBusDevice while it's TypeInfo claims it to
>>> inherit from TYPE_MACHINE. This is an inconsistency which can cause
>>> undefined behavior such as memory corruption.
>>>
>>> Change SiFiveEState to inherit from MachineState since it is registered
>>> as a machine.
>>>
>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>
>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
>To the SiFive maintainers: since this is a bug fix, let's merge it right
>away.

I could repost this particular patch with the three new tags (incl. Fixes) if desired.

Best regards,
Bernhard
>
Re: [PATCH 1/9] hw/riscv/sifive_e: Fix inheritance of SiFiveEState
Posted by Markus Armbruster 3 years, 4 months ago
Bernhard Beschow <shentey@gmail.com> writes:

> Am 20. September 2022 11:36:47 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>Alistair Francis <alistair23@gmail.com> writes:
>>
>>> On Tue, Sep 20, 2022 at 9:18 AM Bernhard Beschow <shentey@gmail.com> wrote:
>>>>
>>>> SiFiveEState inherits from SysBusDevice while it's TypeInfo claims it to
>>>> inherit from TYPE_MACHINE. This is an inconsistency which can cause
>>>> undefined behavior such as memory corruption.
>>>>
>>>> Change SiFiveEState to inherit from MachineState since it is registered
>>>> as a machine.
>>>>
>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>
>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>
>>To the SiFive maintainers: since this is a bug fix, let's merge it right
>>away.
>
> I could repost this particular patch with the three new tags (incl. Fixes) if desired.

Can't hurt, and could help the maintainers.
Re: [PATCH 1/9] hw/riscv/sifive_e: Fix inheritance of SiFiveEState
Posted by B 3 years, 4 months ago

Am 21. September 2022 04:55:02 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>Bernhard Beschow <shentey@gmail.com> writes:
>
>> Am 20. September 2022 11:36:47 UTC schrieb Markus Armbruster <armbru@redhat.com>:
>>>Alistair Francis <alistair23@gmail.com> writes:
>>>
>>>> On Tue, Sep 20, 2022 at 9:18 AM Bernhard Beschow <shentey@gmail.com> wrote:
>>>>>
>>>>> SiFiveEState inherits from SysBusDevice while it's TypeInfo claims it to
>>>>> inherit from TYPE_MACHINE. This is an inconsistency which can cause
>>>>> undefined behavior such as memory corruption.
>>>>>
>>>>> Change SiFiveEState to inherit from MachineState since it is registered
>>>>> as a machine.
>>>>>
>>>>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>>>>
>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>>
>>>To the SiFive maintainers: since this is a bug fix, let's merge it right
>>>away.
>>
>> I could repost this particular patch with the three new tags (incl. Fixes) if desired.
>
>Can't hurt, and could help the maintainers.

[X] Done.

Best regards,
Bernhard