[PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Ivan Griffin posted 1 patch 3 days ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20201016123737.8118-1-ivan.griffin@emdalo.com
Maintainers: Bin Meng <bin.meng@windriver.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <Alistair.Francis@wdc.com>
hw/riscv/microchip_pfsoc.c         | 6 ++++++
include/hw/riscv/microchip_pfsoc.h | 1 +
2 files changed, 7 insertions(+)

[PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Posted by Ivan Griffin 3 days ago
Adding the PolarFire SoC IOSCBCTRL memory region to prevent QEMU
reporting a STORE/AMO Access Fault.

This region is used by the PolarFire SoC port of U-Boot to
interact with the FPGA system controller.

Signed-off-by: Ivan Griffin <ivan.griffin@emdalo.com>
---
 hw/riscv/microchip_pfsoc.c         | 6 ++++++
 include/hw/riscv/microchip_pfsoc.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 4627179cd3..20e1496e3e 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -97,6 +97,7 @@ static const struct MemmapEntry {
     [MICROCHIP_PFSOC_GPIO2] =           { 0x20122000,     0x1000 },
     [MICROCHIP_PFSOC_ENVM_CFG] =        { 0x20200000,     0x1000 },
     [MICROCHIP_PFSOC_ENVM_DATA] =       { 0x20220000,    0x20000 },
+    [MICROCHIP_PFSOC_IOSCB_CTRL] =      { 0x37020000,     0x1000 },
     [MICROCHIP_PFSOC_IOSCB_CFG] =       { 0x37080000,     0x1000 },
     [MICROCHIP_PFSOC_DRAM] =            { 0x80000000,        0x0 },
 };
@@ -341,6 +342,11 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
     create_unimplemented_device("microchip.pfsoc.ioscb.cfg",
         memmap[MICROCHIP_PFSOC_IOSCB_CFG].base,
         memmap[MICROCHIP_PFSOC_IOSCB_CFG].size);
+
+    /* IOSCBCTRL */
+    create_unimplemented_device("microchip.pfsoc.ioscb.ctrl",
+        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].base,
+        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].size);
 }
 
 static void microchip_pfsoc_soc_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
index 8bfc7e1a85..3f1874b162 100644
--- a/include/hw/riscv/microchip_pfsoc.h
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -95,6 +95,7 @@ enum {
     MICROCHIP_PFSOC_ENVM_CFG,
     MICROCHIP_PFSOC_ENVM_DATA,
     MICROCHIP_PFSOC_IOSCB_CFG,
+    MICROCHIP_PFSOC_IOSCB_CTRL,
     MICROCHIP_PFSOC_DRAM,
 };
 
-- 
2.17.1


Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Posted by Alistair Francis 3 days ago
On Fri, Oct 16, 2020 at 8:04 AM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
>
> Adding the PolarFire SoC IOSCBCTRL memory region to prevent QEMU
> reporting a STORE/AMO Access Fault.
>
> This region is used by the PolarFire SoC port of U-Boot to
> interact with the FPGA system controller.
>
> Signed-off-by: Ivan Griffin <ivan.griffin@emdalo.com>
> ---
>  hw/riscv/microchip_pfsoc.c         | 6 ++++++
>  include/hw/riscv/microchip_pfsoc.h | 1 +
>  2 files changed, 7 insertions(+)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index 4627179cd3..20e1496e3e 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -97,6 +97,7 @@ static const struct MemmapEntry {
>      [MICROCHIP_PFSOC_GPIO2] =           { 0x20122000,     0x1000 },
>      [MICROCHIP_PFSOC_ENVM_CFG] =        { 0x20200000,     0x1000 },
>      [MICROCHIP_PFSOC_ENVM_DATA] =       { 0x20220000,    0x20000 },
> +    [MICROCHIP_PFSOC_IOSCB_CTRL] =      { 0x37020000,     0x1000 },

I don't see this in the UG0880 "User Guide PolarFire SoC FPGA
Microprocessor Sub-System" memory map.

Where is this documented?

Alistair

>      [MICROCHIP_PFSOC_IOSCB_CFG] =       { 0x37080000,     0x1000 },
>      [MICROCHIP_PFSOC_DRAM] =            { 0x80000000,        0x0 },
>  };
> @@ -341,6 +342,11 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
>      create_unimplemented_device("microchip.pfsoc.ioscb.cfg",
>          memmap[MICROCHIP_PFSOC_IOSCB_CFG].base,
>          memmap[MICROCHIP_PFSOC_IOSCB_CFG].size);
> +
> +    /* IOSCBCTRL */
> +    create_unimplemented_device("microchip.pfsoc.ioscb.ctrl",
> +        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].base,
> +        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].size);
>  }
>
>  static void microchip_pfsoc_soc_class_init(ObjectClass *oc, void *data)
> diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
> index 8bfc7e1a85..3f1874b162 100644
> --- a/include/hw/riscv/microchip_pfsoc.h
> +++ b/include/hw/riscv/microchip_pfsoc.h
> @@ -95,6 +95,7 @@ enum {
>      MICROCHIP_PFSOC_ENVM_CFG,
>      MICROCHIP_PFSOC_ENVM_DATA,
>      MICROCHIP_PFSOC_IOSCB_CFG,
> +    MICROCHIP_PFSOC_IOSCB_CTRL,
>      MICROCHIP_PFSOC_DRAM,
>  };
>
> --
> 2.17.1
>
>

RE: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Posted by Ivan Griffin 3 days ago
I don't know why it isn't documented in that PDF (or in the register map), but if you check https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.h you'll see the following

```
typedef struct
{
    volatile uint32_t SOFT_RESET;
    volatile uint32_t VDETECTOR;
    volatile uint32_t TVS_CONTROL;
    volatile uint32_t TVS_TEMP_A;
    volatile uint32_t TVS_TEMP_B;
    volatile uint32_t TVS_TEMP_C;
    volatile uint32_t TVS_VOLT_A;
    volatile uint32_t TVS_VOLT_B;
    volatile uint32_t TVS_VOLT_C;
    volatile uint32_t TVS_OUTPUT0;
    volatile uint32_t TVS_OUTPUT1;
    volatile uint32_t TVS_TRIGGER;
    volatile uint32_t TRIM_VDET1P05;
    volatile uint32_t TRIM_VDET1P8;
    volatile uint32_t TRIM_VDET2P5;
    volatile uint32_t TRIM_TVS;
    volatile uint32_t TRIM_GDET1P05;
    volatile uint32_t RESERVED0;
    volatile uint32_t RESERVED1;
    volatile uint32_t RESERVED2;
    volatile uint32_t SERVICES_CR;
    volatile uint32_t SERVICES_SR;
    volatile uint32_t USER_DETECTOR_SR;
    volatile uint32_t USER_DETECTOR_CR;
    volatile uint32_t MSS_SPI_CR;

} SCBCTRL_TypeDef;

#define MSS_SCBCTRL                    ((SCBCTRL_TypeDef*) (0x37020000UL))

/*2kB bytes long mailbox.*/
#define MSS_SCBMAILBOX                 ((uint32_t*) (0x37020800UL))
```

And in https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.c you'll see MSS_SCB and MSS_SCBMAILBOX used in many places to interact with the FPGA system controller to perform various services.


Cheers,
Ivan


-----Original Message-----
From: Alistair Francis <alistair23@gmail.com> 
Sent: Friday 16 October 2020 17:08
To: Ivan Griffin <ivan.griffin@emdalo.com>
Cc: Bin Meng <bin.meng@windriver.com>; QEMU Trivial <qemu-trivial@nongnu.org>; open list:RISC-V <qemu-riscv@nongnu.org>; qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>
Subject: Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

On Fri, Oct 16, 2020 at 8:04 AM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
>
> Adding the PolarFire SoC IOSCBCTRL memory region to prevent QEMU 
> reporting a STORE/AMO Access Fault.
>
> This region is used by the PolarFire SoC port of U-Boot to interact 
> with the FPGA system controller.
>
> Signed-off-by: Ivan Griffin <ivan.griffin@emdalo.com>
> ---
>  hw/riscv/microchip_pfsoc.c         | 6 ++++++
>  include/hw/riscv/microchip_pfsoc.h | 1 +
>  2 files changed, 7 insertions(+)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c 
> index 4627179cd3..20e1496e3e 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -97,6 +97,7 @@ static const struct MemmapEntry {
>      [MICROCHIP_PFSOC_GPIO2] =           { 0x20122000,     0x1000 },
>      [MICROCHIP_PFSOC_ENVM_CFG] =        { 0x20200000,     0x1000 },
>      [MICROCHIP_PFSOC_ENVM_DATA] =       { 0x20220000,    0x20000 },
> +    [MICROCHIP_PFSOC_IOSCB_CTRL] =      { 0x37020000,     0x1000 },

I don't see this in the UG0880 "User Guide PolarFire SoC FPGA Microprocessor Sub-System" memory map.

Where is this documented?

Alistair

>      [MICROCHIP_PFSOC_IOSCB_CFG] =       { 0x37080000,     0x1000 },
>      [MICROCHIP_PFSOC_DRAM] =            { 0x80000000,        0x0 },
>  };
> @@ -341,6 +342,11 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
>      create_unimplemented_device("microchip.pfsoc.ioscb.cfg",
>          memmap[MICROCHIP_PFSOC_IOSCB_CFG].base,
>          memmap[MICROCHIP_PFSOC_IOSCB_CFG].size);
> +
> +    /* IOSCBCTRL */
> +    create_unimplemented_device("microchip.pfsoc.ioscb.ctrl",
> +        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].base,
> +        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].size);
>  }
>
>  static void microchip_pfsoc_soc_class_init(ObjectClass *oc, void 
> *data) diff --git a/include/hw/riscv/microchip_pfsoc.h 
> b/include/hw/riscv/microchip_pfsoc.h
> index 8bfc7e1a85..3f1874b162 100644
> --- a/include/hw/riscv/microchip_pfsoc.h
> +++ b/include/hw/riscv/microchip_pfsoc.h
> @@ -95,6 +95,7 @@ enum {
>      MICROCHIP_PFSOC_ENVM_CFG,
>      MICROCHIP_PFSOC_ENVM_DATA,
>      MICROCHIP_PFSOC_IOSCB_CFG,
> +    MICROCHIP_PFSOC_IOSCB_CTRL,
>      MICROCHIP_PFSOC_DRAM,
>  };
>
> --
> 2.17.1
>
>

Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Posted by Bin Meng 1 day ago
Hi Ivan,

On Sat, Oct 17, 2020 at 12:31 AM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
>
> I don't know why it isn't documented in that PDF (or in the register map), but if you check https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.h you'll see the following
>
> ```
> typedef struct
> {
>     volatile uint32_t SOFT_RESET;
>     volatile uint32_t VDETECTOR;
>     volatile uint32_t TVS_CONTROL;
>     volatile uint32_t TVS_TEMP_A;
>     volatile uint32_t TVS_TEMP_B;
>     volatile uint32_t TVS_TEMP_C;
>     volatile uint32_t TVS_VOLT_A;
>     volatile uint32_t TVS_VOLT_B;
>     volatile uint32_t TVS_VOLT_C;
>     volatile uint32_t TVS_OUTPUT0;
>     volatile uint32_t TVS_OUTPUT1;
>     volatile uint32_t TVS_TRIGGER;
>     volatile uint32_t TRIM_VDET1P05;
>     volatile uint32_t TRIM_VDET1P8;
>     volatile uint32_t TRIM_VDET2P5;
>     volatile uint32_t TRIM_TVS;
>     volatile uint32_t TRIM_GDET1P05;
>     volatile uint32_t RESERVED0;
>     volatile uint32_t RESERVED1;
>     volatile uint32_t RESERVED2;
>     volatile uint32_t SERVICES_CR;
>     volatile uint32_t SERVICES_SR;
>     volatile uint32_t USER_DETECTOR_SR;
>     volatile uint32_t USER_DETECTOR_CR;
>     volatile uint32_t MSS_SPI_CR;
>
> } SCBCTRL_TypeDef;
>
> #define MSS_SCBCTRL                    ((SCBCTRL_TypeDef*) (0x37020000UL))
>
> /*2kB bytes long mailbox.*/
> #define MSS_SCBMAILBOX                 ((uint32_t*) (0x37020800UL))
> ```
>
> And in https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.c you'll see MSS_SCB and MSS_SCBMAILBOX used in many places to interact with the FPGA system controller to perform various services.

It's actually documented, but not in the PDF file. I also spent some
time locating the doc when I do the DDR controller modeling work.

See Register Map/PF_SoC_RegMap_V1_1/MPFS250T/pfsoc_control_scb.htm in
https://www.microsemi.com/document-portal/doc_download/1244581-polarfire-soc-register-map

Regards,
Bin

Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Posted by Ivan Griffin 18 hours ago
Hi Bin,

Well spotted with the register map. I grepped it for 0x37020000 and didn't find it, but it seems the address (incorrectly) is 0x07020000 in the documentation.

Thanks,
Ivan.

________________________________
From: Bin Meng <bmeng.cn@gmail.com>
Sent: Monday 19 October 2020 03:05
To: Ivan Griffin <ivan.griffin@emdalo.com>
Cc: Alistair Francis <alistair23@gmail.com>; QEMU Trivial <qemu-trivial@nongnu.org>; Bin Meng <bin.meng@windriver.com>; open list:RISC-V <qemu-riscv@nongnu.org>; qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>
Subject: Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Hi Ivan,

On Sat, Oct 17, 2020 at 12:31 AM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
>
> I don't know why it isn't documented in that PDF (or in the register map), but if you check https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.h you'll see the following
>
> ```
> typedef struct
> {
>     volatile uint32_t SOFT_RESET;
>     volatile uint32_t VDETECTOR;
>     volatile uint32_t TVS_CONTROL;
>     volatile uint32_t TVS_TEMP_A;
>     volatile uint32_t TVS_TEMP_B;
>     volatile uint32_t TVS_TEMP_C;
>     volatile uint32_t TVS_VOLT_A;
>     volatile uint32_t TVS_VOLT_B;
>     volatile uint32_t TVS_VOLT_C;
>     volatile uint32_t TVS_OUTPUT0;
>     volatile uint32_t TVS_OUTPUT1;
>     volatile uint32_t TVS_TRIGGER;
>     volatile uint32_t TRIM_VDET1P05;
>     volatile uint32_t TRIM_VDET1P8;
>     volatile uint32_t TRIM_VDET2P5;
>     volatile uint32_t TRIM_TVS;
>     volatile uint32_t TRIM_GDET1P05;
>     volatile uint32_t RESERVED0;
>     volatile uint32_t RESERVED1;
>     volatile uint32_t RESERVED2;
>     volatile uint32_t SERVICES_CR;
>     volatile uint32_t SERVICES_SR;
>     volatile uint32_t USER_DETECTOR_SR;
>     volatile uint32_t USER_DETECTOR_CR;
>     volatile uint32_t MSS_SPI_CR;
>
> } SCBCTRL_TypeDef;
>
> #define MSS_SCBCTRL                    ((SCBCTRL_TypeDef*) (0x37020000UL))
>
> /*2kB bytes long mailbox.*/
> #define MSS_SCBMAILBOX                 ((uint32_t*) (0x37020800UL))
> ```
>
> And in https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.c you'll see MSS_SCB and MSS_SCBMAILBOX used in many places to interact with the FPGA system controller to perform various services.

It's actually documented, but not in the PDF file. I also spent some
time locating the doc when I do the DDR controller modeling work.

See Register Map/PF_SoC_RegMap_V1_1/MPFS250T/pfsoc_control_scb.htm in
https://www.microsemi.com/document-portal/doc_download/1244581-polarfire-soc-register-map

Regards,
Bin

Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Posted by Bin Meng 18 hours ago
Hi Ivan,

On Mon, Oct 19, 2020 at 4:17 PM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
>
> Hi Bin,
>
> Well spotted with the register map. I grepped it for 0x37020000 and didn't find it, but it seems the address (incorrectly) is 0x07020000 in the documentation.
>

I believe the documented offset 0x07020000 is the offset into the
IOSCB block, not the final one that is mapped into the system memory.

Regards,
Bin

> ________________________________
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: Monday 19 October 2020 03:05
> To: Ivan Griffin <ivan.griffin@emdalo.com>
> Cc: Alistair Francis <alistair23@gmail.com>; QEMU Trivial <qemu-trivial@nongnu.org>; Bin Meng <bin.meng@windriver.com>; open list:RISC-V <qemu-riscv@nongnu.org>; qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>
> Subject: Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry
>
> Hi Ivan,
>
> On Sat, Oct 17, 2020 at 12:31 AM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
> >
> > I don't know why it isn't documented in that PDF (or in the register map), but if you check https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.h you'll see the following
> >
> > ```
> > typedef struct
> > {
> >     volatile uint32_t SOFT_RESET;
> >     volatile uint32_t VDETECTOR;
> >     volatile uint32_t TVS_CONTROL;
> >     volatile uint32_t TVS_TEMP_A;
> >     volatile uint32_t TVS_TEMP_B;
> >     volatile uint32_t TVS_TEMP_C;
> >     volatile uint32_t TVS_VOLT_A;
> >     volatile uint32_t TVS_VOLT_B;
> >     volatile uint32_t TVS_VOLT_C;
> >     volatile uint32_t TVS_OUTPUT0;
> >     volatile uint32_t TVS_OUTPUT1;
> >     volatile uint32_t TVS_TRIGGER;
> >     volatile uint32_t TRIM_VDET1P05;
> >     volatile uint32_t TRIM_VDET1P8;
> >     volatile uint32_t TRIM_VDET2P5;
> >     volatile uint32_t TRIM_TVS;
> >     volatile uint32_t TRIM_GDET1P05;
> >     volatile uint32_t RESERVED0;
> >     volatile uint32_t RESERVED1;
> >     volatile uint32_t RESERVED2;
> >     volatile uint32_t SERVICES_CR;
> >     volatile uint32_t SERVICES_SR;
> >     volatile uint32_t USER_DETECTOR_SR;
> >     volatile uint32_t USER_DETECTOR_CR;
> >     volatile uint32_t MSS_SPI_CR;
> >
> > } SCBCTRL_TypeDef;
> >
> > #define MSS_SCBCTRL                    ((SCBCTRL_TypeDef*) (0x37020000UL))
> >
> > /*2kB bytes long mailbox.*/
> > #define MSS_SCBMAILBOX                 ((uint32_t*) (0x37020800UL))
> > ```
> >
> > And in https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.c you'll see MSS_SCB and MSS_SCBMAILBOX used in many places to interact with the FPGA system controller to perform various services.
>
> It's actually documented, but not in the PDF file. I also spent some
> time locating the doc when I do the DDR controller modeling work.
>
> See Register Map/PF_SoC_RegMap_V1_1/MPFS250T/pfsoc_control_scb.htm in
> https://www.microsemi.com/document-portal/doc_download/1244581-polarfire-soc-register-map

Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Posted by Ivan Griffin 18 hours ago
Yes, it could be.

But the "Physical Address" column, in my mind, should be the 0x37020000 address.  At least when I check other peripherals, their physical address values in the register map HTML corresponds to the source code.

Cheers,
Ivan.

________________________________
From: Bin Meng <bmeng.cn@gmail.com>
Sent: Monday 19 October 2020 09:38
To: Ivan Griffin <ivan.griffin@emdalo.com>
Cc: Alistair Francis <alistair23@gmail.com>; QEMU Trivial <qemu-trivial@nongnu.org>; open list:RISC-V <qemu-riscv@nongnu.org>; qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>
Subject: Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Hi Ivan,

On Mon, Oct 19, 2020 at 4:17 PM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
>
> Hi Bin,
>
> Well spotted with the register map. I grepped it for 0x37020000 and didn't find it, but it seems the address (incorrectly) is 0x07020000 in the documentation.
>

I believe the documented offset 0x07020000 is the offset into the
IOSCB block, not the final one that is mapped into the system memory.

Regards,
Bin

> ________________________________
> From: Bin Meng <bmeng.cn@gmail.com>
> Sent: Monday 19 October 2020 03:05
> To: Ivan Griffin <ivan.griffin@emdalo.com>
> Cc: Alistair Francis <alistair23@gmail.com>; QEMU Trivial <qemu-trivial@nongnu.org>; Bin Meng <bin.meng@windriver.com>; open list:RISC-V <qemu-riscv@nongnu.org>; qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>
> Subject: Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry
>
> Hi Ivan,
>
> On Sat, Oct 17, 2020 at 12:31 AM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
> >
> > I don't know why it isn't documented in that PDF (or in the register map), but if you check https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.h you'll see the following
> >
> > ```
> > typedef struct
> > {
> >     volatile uint32_t SOFT_RESET;
> >     volatile uint32_t VDETECTOR;
> >     volatile uint32_t TVS_CONTROL;
> >     volatile uint32_t TVS_TEMP_A;
> >     volatile uint32_t TVS_TEMP_B;
> >     volatile uint32_t TVS_TEMP_C;
> >     volatile uint32_t TVS_VOLT_A;
> >     volatile uint32_t TVS_VOLT_B;
> >     volatile uint32_t TVS_VOLT_C;
> >     volatile uint32_t TVS_OUTPUT0;
> >     volatile uint32_t TVS_OUTPUT1;
> >     volatile uint32_t TVS_TRIGGER;
> >     volatile uint32_t TRIM_VDET1P05;
> >     volatile uint32_t TRIM_VDET1P8;
> >     volatile uint32_t TRIM_VDET2P5;
> >     volatile uint32_t TRIM_TVS;
> >     volatile uint32_t TRIM_GDET1P05;
> >     volatile uint32_t RESERVED0;
> >     volatile uint32_t RESERVED1;
> >     volatile uint32_t RESERVED2;
> >     volatile uint32_t SERVICES_CR;
> >     volatile uint32_t SERVICES_SR;
> >     volatile uint32_t USER_DETECTOR_SR;
> >     volatile uint32_t USER_DETECTOR_CR;
> >     volatile uint32_t MSS_SPI_CR;
> >
> > } SCBCTRL_TypeDef;
> >
> > #define MSS_SCBCTRL                    ((SCBCTRL_TypeDef*) (0x37020000UL))
> >
> > /*2kB bytes long mailbox.*/
> > #define MSS_SCBMAILBOX                 ((uint32_t*) (0x37020800UL))
> > ```
> >
> > And in https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.c you'll see MSS_SCB and MSS_SCBMAILBOX used in many places to interact with the FPGA system controller to perform various services.
>
> It's actually documented, but not in the PDF file. I also spent some
> time locating the doc when I do the DDR controller modeling work.
>
> See Register Map/PF_SoC_RegMap_V1_1/MPFS250T/pfsoc_control_scb.htm in
> https://www.microsemi.com/document-portal/doc_download/1244581-polarfire-soc-register-map

Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Posted by Alistair Francis 3 days ago
On Fri, Oct 16, 2020 at 9:31 AM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
>
> I don't know why it isn't documented in that PDF (or in the register map), but if you check https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.h you'll see the following
>
> ```
> typedef struct
> {
>     volatile uint32_t SOFT_RESET;
>     volatile uint32_t VDETECTOR;
>     volatile uint32_t TVS_CONTROL;
>     volatile uint32_t TVS_TEMP_A;
>     volatile uint32_t TVS_TEMP_B;
>     volatile uint32_t TVS_TEMP_C;
>     volatile uint32_t TVS_VOLT_A;
>     volatile uint32_t TVS_VOLT_B;
>     volatile uint32_t TVS_VOLT_C;
>     volatile uint32_t TVS_OUTPUT0;
>     volatile uint32_t TVS_OUTPUT1;
>     volatile uint32_t TVS_TRIGGER;
>     volatile uint32_t TRIM_VDET1P05;
>     volatile uint32_t TRIM_VDET1P8;
>     volatile uint32_t TRIM_VDET2P5;
>     volatile uint32_t TRIM_TVS;
>     volatile uint32_t TRIM_GDET1P05;
>     volatile uint32_t RESERVED0;
>     volatile uint32_t RESERVED1;
>     volatile uint32_t RESERVED2;
>     volatile uint32_t SERVICES_CR;
>     volatile uint32_t SERVICES_SR;
>     volatile uint32_t USER_DETECTOR_SR;
>     volatile uint32_t USER_DETECTOR_CR;
>     volatile uint32_t MSS_SPI_CR;
>
> } SCBCTRL_TypeDef;
>
> #define MSS_SCBCTRL                    ((SCBCTRL_TypeDef*) (0x37020000UL))
>
> /*2kB bytes long mailbox.*/
> #define MSS_SCBMAILBOX                 ((uint32_t*) (0x37020800UL))
> ```
>
> And in https://github.com/polarfire-soc/polarfire-soc-bare-metal-library/blob/master/src/platform/drivers/mss_sys_services/mss_sys_services.c you'll see MSS_SCB and MSS_SCBMAILBOX used in many places to interact with the FPGA system controller to perform various services.

Ok, so the memory map exists, but not documented in the official documentation.

In this case it's fine to add. Can you add a comment though saying
that it's not documented in the official documentation though?
Otherwise it will confuse developers.

Alistair

>
>
> Cheers,
> Ivan
>
>
> -----Original Message-----
> From: Alistair Francis <alistair23@gmail.com>
> Sent: Friday 16 October 2020 17:08
> To: Ivan Griffin <ivan.griffin@emdalo.com>
> Cc: Bin Meng <bin.meng@windriver.com>; QEMU Trivial <qemu-trivial@nongnu.org>; open list:RISC-V <qemu-riscv@nongnu.org>; qemu-devel@nongnu.org Developers <qemu-devel@nongnu.org>
> Subject: Re: [PATCH] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry
>
> On Fri, Oct 16, 2020 at 8:04 AM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
> >
> > Adding the PolarFire SoC IOSCBCTRL memory region to prevent QEMU
> > reporting a STORE/AMO Access Fault.
> >
> > This region is used by the PolarFire SoC port of U-Boot to interact
> > with the FPGA system controller.
> >
> > Signed-off-by: Ivan Griffin <ivan.griffin@emdalo.com>
> > ---
> >  hw/riscv/microchip_pfsoc.c         | 6 ++++++
> >  include/hw/riscv/microchip_pfsoc.h | 1 +
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> > index 4627179cd3..20e1496e3e 100644
> > --- a/hw/riscv/microchip_pfsoc.c
> > +++ b/hw/riscv/microchip_pfsoc.c
> > @@ -97,6 +97,7 @@ static const struct MemmapEntry {
> >      [MICROCHIP_PFSOC_GPIO2] =           { 0x20122000,     0x1000 },
> >      [MICROCHIP_PFSOC_ENVM_CFG] =        { 0x20200000,     0x1000 },
> >      [MICROCHIP_PFSOC_ENVM_DATA] =       { 0x20220000,    0x20000 },
> > +    [MICROCHIP_PFSOC_IOSCB_CTRL] =      { 0x37020000,     0x1000 },
>
> I don't see this in the UG0880 "User Guide PolarFire SoC FPGA Microprocessor Sub-System" memory map.
>
> Where is this documented?
>
> Alistair
>
> >      [MICROCHIP_PFSOC_IOSCB_CFG] =       { 0x37080000,     0x1000 },
> >      [MICROCHIP_PFSOC_DRAM] =            { 0x80000000,        0x0 },
> >  };
> > @@ -341,6 +342,11 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
> >      create_unimplemented_device("microchip.pfsoc.ioscb.cfg",
> >          memmap[MICROCHIP_PFSOC_IOSCB_CFG].base,
> >          memmap[MICROCHIP_PFSOC_IOSCB_CFG].size);
> > +
> > +    /* IOSCBCTRL */
> > +    create_unimplemented_device("microchip.pfsoc.ioscb.ctrl",
> > +        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].base,
> > +        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].size);
> >  }
> >
> >  static void microchip_pfsoc_soc_class_init(ObjectClass *oc, void
> > *data) diff --git a/include/hw/riscv/microchip_pfsoc.h
> > b/include/hw/riscv/microchip_pfsoc.h
> > index 8bfc7e1a85..3f1874b162 100644
> > --- a/include/hw/riscv/microchip_pfsoc.h
> > +++ b/include/hw/riscv/microchip_pfsoc.h
> > @@ -95,6 +95,7 @@ enum {
> >      MICROCHIP_PFSOC_ENVM_CFG,
> >      MICROCHIP_PFSOC_ENVM_DATA,
> >      MICROCHIP_PFSOC_IOSCB_CFG,
> > +    MICROCHIP_PFSOC_IOSCB_CTRL,
> >      MICROCHIP_PFSOC_DRAM,
> >  };
> >
> > --
> > 2.17.1
> >
> >

[PATCH v2] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Posted by Ivan Griffin 3 days ago
Adding the PolarFire SoC IOSCBCTRL memory region to prevent QEMU
reporting a STORE/AMO Access Fault.

This region is used by the PolarFire SoC port of U-Boot to
interact with the FPGA system controller.

Signed-off-by: Ivan Griffin <ivan.griffin@emdalo.com>
---
 hw/riscv/microchip_pfsoc.c         | 10 ++++++++++
 include/hw/riscv/microchip_pfsoc.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 4627179cd3..9aaa276ee2 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -97,6 +97,7 @@ static const struct MemmapEntry {
     [MICROCHIP_PFSOC_GPIO2] =           { 0x20122000,     0x1000 },
     [MICROCHIP_PFSOC_ENVM_CFG] =        { 0x20200000,     0x1000 },
     [MICROCHIP_PFSOC_ENVM_DATA] =       { 0x20220000,    0x20000 },
+    [MICROCHIP_PFSOC_IOSCB_CTRL] =      { 0x37020000,     0x1000 },
     [MICROCHIP_PFSOC_IOSCB_CFG] =       { 0x37080000,     0x1000 },
     [MICROCHIP_PFSOC_DRAM] =            { 0x80000000,        0x0 },
 };
@@ -341,6 +342,15 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
     create_unimplemented_device("microchip.pfsoc.ioscb.cfg",
         memmap[MICROCHIP_PFSOC_IOSCB_CFG].base,
         memmap[MICROCHIP_PFSOC_IOSCB_CFG].size);
+
+    /* IOSCBCTRL
+     *
+     * These registers are not documented in the official documentation
+     * but used by the polarfire-soc-bare-meta-library code
+     */
+    create_unimplemented_device("microchip.pfsoc.ioscb.ctrl",
+        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].base,
+        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].size);
 }
 
 static void microchip_pfsoc_soc_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
index 8bfc7e1a85..3f1874b162 100644
--- a/include/hw/riscv/microchip_pfsoc.h
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -95,6 +95,7 @@ enum {
     MICROCHIP_PFSOC_ENVM_CFG,
     MICROCHIP_PFSOC_ENVM_DATA,
     MICROCHIP_PFSOC_IOSCB_CFG,
+    MICROCHIP_PFSOC_IOSCB_CTRL,
     MICROCHIP_PFSOC_DRAM,
 };
 
-- 
2.17.1


Re: [PATCH v2] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Posted by Bin Meng 1 day ago
Hi Ivan,

On Sat, Oct 17, 2020 at 1:10 AM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
>
> Adding the PolarFire SoC IOSCBCTRL memory region to prevent QEMU
> reporting a STORE/AMO Access Fault.
>
> This region is used by the PolarFire SoC port of U-Boot to
> interact with the FPGA system controller.
>
> Signed-off-by: Ivan Griffin <ivan.griffin@emdalo.com>
> ---
>  hw/riscv/microchip_pfsoc.c         | 10 ++++++++++
>  include/hw/riscv/microchip_pfsoc.h |  1 +
>  2 files changed, 11 insertions(+)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index 4627179cd3..9aaa276ee2 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -97,6 +97,7 @@ static const struct MemmapEntry {
>      [MICROCHIP_PFSOC_GPIO2] =           { 0x20122000,     0x1000 },
>      [MICROCHIP_PFSOC_ENVM_CFG] =        { 0x20200000,     0x1000 },
>      [MICROCHIP_PFSOC_ENVM_DATA] =       { 0x20220000,    0x20000 },
> +    [MICROCHIP_PFSOC_IOSCB_CTRL] =      { 0x37020000,     0x1000 },
>      [MICROCHIP_PFSOC_IOSCB_CFG] =       { 0x37080000,     0x1000 },
>      [MICROCHIP_PFSOC_DRAM] =            { 0x80000000,        0x0 },
>  };
> @@ -341,6 +342,15 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
>      create_unimplemented_device("microchip.pfsoc.ioscb.cfg",
>          memmap[MICROCHIP_PFSOC_IOSCB_CFG].base,
>          memmap[MICROCHIP_PFSOC_IOSCB_CFG].size);
> +
> +    /* IOSCBCTRL
> +     *
> +     * These registers are not documented in the official documentation
> +     * but used by the polarfire-soc-bare-meta-library code
> +     */
> +    create_unimplemented_device("microchip.pfsoc.ioscb.ctrl",
> +        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].base,
> +        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].size);
>  }
>
>  static void microchip_pfsoc_soc_class_init(ObjectClass *oc, void *data)
> diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
> index 8bfc7e1a85..3f1874b162 100644
> --- a/include/hw/riscv/microchip_pfsoc.h
> +++ b/include/hw/riscv/microchip_pfsoc.h
> @@ -95,6 +95,7 @@ enum {
>      MICROCHIP_PFSOC_ENVM_CFG,
>      MICROCHIP_PFSOC_ENVM_DATA,
>      MICROCHIP_PFSOC_IOSCB_CFG,
> +    MICROCHIP_PFSOC_IOSCB_CTRL,
>      MICROCHIP_PFSOC_DRAM,
>  };

Thank you for the patch!

I am currently adding the DDR controller modeling support to PolarFire
SoC which will cover this memory map. With my patch series, your patch
is no longer needed.

Regards,
Bin

Re: [PATCH v2] hw/riscv: microchip_pfsoc: IOSCBCTRL memmap entry

Posted by Alistair Francis 3 days ago
On Fri, Oct 16, 2020 at 10:10 AM Ivan Griffin <ivan.griffin@emdalo.com> wrote:
>
> Adding the PolarFire SoC IOSCBCTRL memory region to prevent QEMU
> reporting a STORE/AMO Access Fault.
>
> This region is used by the PolarFire SoC port of U-Boot to
> interact with the FPGA system controller.
>
> Signed-off-by: Ivan Griffin <ivan.griffin@emdalo.com>

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

Alistair

> ---
>  hw/riscv/microchip_pfsoc.c         | 10 ++++++++++
>  include/hw/riscv/microchip_pfsoc.h |  1 +
>  2 files changed, 11 insertions(+)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index 4627179cd3..9aaa276ee2 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -97,6 +97,7 @@ static const struct MemmapEntry {
>      [MICROCHIP_PFSOC_GPIO2] =           { 0x20122000,     0x1000 },
>      [MICROCHIP_PFSOC_ENVM_CFG] =        { 0x20200000,     0x1000 },
>      [MICROCHIP_PFSOC_ENVM_DATA] =       { 0x20220000,    0x20000 },
> +    [MICROCHIP_PFSOC_IOSCB_CTRL] =      { 0x37020000,     0x1000 },
>      [MICROCHIP_PFSOC_IOSCB_CFG] =       { 0x37080000,     0x1000 },
>      [MICROCHIP_PFSOC_DRAM] =            { 0x80000000,        0x0 },
>  };
> @@ -341,6 +342,15 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
>      create_unimplemented_device("microchip.pfsoc.ioscb.cfg",
>          memmap[MICROCHIP_PFSOC_IOSCB_CFG].base,
>          memmap[MICROCHIP_PFSOC_IOSCB_CFG].size);
> +
> +    /* IOSCBCTRL
> +     *
> +     * These registers are not documented in the official documentation
> +     * but used by the polarfire-soc-bare-meta-library code
> +     */
> +    create_unimplemented_device("microchip.pfsoc.ioscb.ctrl",
> +        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].base,
> +        memmap[MICROCHIP_PFSOC_IOSCB_CTRL].size);
>  }
>
>  static void microchip_pfsoc_soc_class_init(ObjectClass *oc, void *data)
> diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
> index 8bfc7e1a85..3f1874b162 100644
> --- a/include/hw/riscv/microchip_pfsoc.h
> +++ b/include/hw/riscv/microchip_pfsoc.h
> @@ -95,6 +95,7 @@ enum {
>      MICROCHIP_PFSOC_ENVM_CFG,
>      MICROCHIP_PFSOC_ENVM_DATA,
>      MICROCHIP_PFSOC_IOSCB_CFG,
> +    MICROCHIP_PFSOC_IOSCB_CTRL,
>      MICROCHIP_PFSOC_DRAM,
>  };
>
> --
> 2.17.1
>