* [PATCH v2] hw/m68k/mcf5208: add support for reset
@ 2024-03-08 9:08 Angelo Dureghello
2024-03-09 9:06 ` Thomas Huth
0 siblings, 1 reply; 3+ messages in thread
From: Angelo Dureghello @ 2024-03-08 9:08 UTC (permalink / raw)
To: huth; +Cc: qemu-devel, Angelo Dureghello
Add reset support for mcf5208.
Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
---
hw/m68k/mcf5208.c | 49 ++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 46 insertions(+), 3 deletions(-)
diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index 0cfb806c20..d8a38274d0 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -40,6 +40,8 @@
#define PCSR_PRE_SHIFT 8
#define PCSR_PRE_MASK 0x0f00
+#define RCR_SOFTRST 0x80
+
typedef struct {
MemoryRegion iomem;
qemu_irq irq;
@@ -185,13 +187,54 @@ static const MemoryRegionOps m5208_sys_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic)
+static uint64_t m5208_rcm_read(void *opaque, hwaddr addr,
+ unsigned size)
+{
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
+ __func__, addr);
+ return 0;
+}
+
+static void m5208_rcm_write(void *opaque, hwaddr addr,
+ uint64_t value, unsigned size)
+{
+ M68kCPU *cpu = opaque;
+ CPUState *cs = CPU(cpu);
+ switch (addr) {
+ case 0x0: /* RCR */
+ if (value & RCR_SOFTRST) {
+ cpu_reset(cs);
+ cpu->env.aregs[7] = ldl_phys(cs->as, 0);
+ cpu->env.pc = ldl_phys(cs->as, 4);
+ }
+ break;
+ default:
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
+ __func__, addr);
+ break;
+ }
+}
+
+static const MemoryRegionOps m5208_rcm_ops = {
+ .read = m5208_rcm_read,
+ .write = m5208_rcm_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic,
+ M68kCPU *cpu)
{
- MemoryRegion *iomem = g_new(MemoryRegion, 1);
+ MemoryRegion *iomem;
m5208_timer_state *s;
int i;
+ /* RCM */
+ iomem = g_new(MemoryRegion, 1);
+ memory_region_init_io(iomem, NULL, &m5208_rcm_ops, cpu,
+ "m5208-rcm", 0x00000080);
+ memory_region_add_subregion(address_space, 0xfc0a0000, iomem);
/* SDRAMC. */
+ iomem = g_new(MemoryRegion, 1);
memory_region_init_io(iomem, NULL, &m5208_sys_ops, NULL, "m5208-sys", 0x00004000);
memory_region_add_subregion(address_space, 0xfc0a8000, iomem);
/* Timers. */
@@ -265,7 +308,7 @@ static void mcf5208evb_init(MachineState *machine)
mcf_uart_create_mmap(0xfc064000, pic[27], serial_hd(1));
mcf_uart_create_mmap(0xfc068000, pic[28], serial_hd(2));
- mcf5208_sys_init(address_space_mem, pic);
+ mcf5208_sys_init(address_space_mem, pic, cpu);
mcf_fec_init(address_space_mem, 0xfc030000, pic + 36);
--
2.44.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] hw/m68k/mcf5208: add support for reset
2024-03-08 9:08 [PATCH v2] hw/m68k/mcf5208: add support for reset Angelo Dureghello
@ 2024-03-09 9:06 ` Thomas Huth
2024-03-09 9:30 ` Angelo Dureghello
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Huth @ 2024-03-09 9:06 UTC (permalink / raw)
To: Angelo Dureghello; +Cc: qemu-devel
Am Fri, 8 Mar 2024 10:08:12 +0100
schrieb Angelo Dureghello <angelo@kernel-space.org>:
> Add reset support for mcf5208.
>
> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
> ---
> hw/m68k/mcf5208.c | 49 ++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
> index 0cfb806c20..d8a38274d0 100644
> --- a/hw/m68k/mcf5208.c
> +++ b/hw/m68k/mcf5208.c
> @@ -40,6 +40,8 @@
> #define PCSR_PRE_SHIFT 8
> #define PCSR_PRE_MASK 0x0f00
>
> +#define RCR_SOFTRST 0x80
> +
> typedef struct {
> MemoryRegion iomem;
> qemu_irq irq;
> @@ -185,13 +187,54 @@ static const MemoryRegionOps m5208_sys_ops = {
> .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> -static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic)
> +static uint64_t m5208_rcm_read(void *opaque, hwaddr addr,
> + unsigned size)
> +{
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
> + __func__, addr);
I'd maybe rather not print this message if the guest tries to read from RCR
or RSR - reading back a 0 should be fine here without logging a "bad
offset".
> + return 0;
> +}
> +
> +static void m5208_rcm_write(void *opaque, hwaddr addr,
> + uint64_t value, unsigned size)
> +{
> + M68kCPU *cpu = opaque;
> + CPUState *cs = CPU(cpu);
> + switch (addr) {
> + case 0x0: /* RCR */
> + if (value & RCR_SOFTRST) {
> + cpu_reset(cs);
> + cpu->env.aregs[7] = ldl_phys(cs->as, 0);
> + cpu->env.pc = ldl_phys(cs->as, 4);
> + }
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
> + __func__, addr);
> + break;
> + }
> +}
> +
> +static const MemoryRegionOps m5208_rcm_ops = {
> + .read = m5208_rcm_read,
> + .write = m5208_rcm_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic,
> + M68kCPU *cpu)
> {
> - MemoryRegion *iomem = g_new(MemoryRegion, 1);
> + MemoryRegion *iomem;
I think it's more common coding style in the QEMU sources to have separate
variables for separate MemoryRegions instead of re-using one variable for
separate MemoryRegions like you do it below. See e.g.
microchip_pfsoc_soc_realize() in hw/riscv/microchip_pfsoc.c.
> m5208_timer_state *s;
> int i;
>
> + /* RCM */
> + iomem = g_new(MemoryRegion, 1);
> + memory_region_init_io(iomem, NULL, &m5208_rcm_ops, cpu,
> + "m5208-rcm", 0x00000080);
> + memory_region_add_subregion(address_space, 0xfc0a0000, iomem);
> /* SDRAMC. */
> + iomem = g_new(MemoryRegion, 1);
> memory_region_init_io(iomem, NULL, &m5208_sys_ops, NULL, "m5208-sys", 0x00004000);
> memory_region_add_subregion(address_space, 0xfc0a8000, iomem);
> /* Timers. */
> @@ -265,7 +308,7 @@ static void mcf5208evb_init(MachineState *machine)
> mcf_uart_create_mmap(0xfc064000, pic[27], serial_hd(1));
> mcf_uart_create_mmap(0xfc068000, pic[28], serial_hd(2));
>
> - mcf5208_sys_init(address_space_mem, pic);
> + mcf5208_sys_init(address_space_mem, pic, cpu);
>
> mcf_fec_init(address_space_mem, 0xfc030000, pic + 36);
All in all, patch looks fine to me ... if you could update the above two
nits by Monday, I can try to get this merged before the next soft freeze of
QEMU (on Tuesday).
Thanks,
Thomas
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] hw/m68k/mcf5208: add support for reset
2024-03-09 9:06 ` Thomas Huth
@ 2024-03-09 9:30 ` Angelo Dureghello
0 siblings, 0 replies; 3+ messages in thread
From: Angelo Dureghello @ 2024-03-09 9:30 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel
Hi Thomas,
On 09/03/24 10:06 AM, Thomas Huth wrote:
> Am Fri, 8 Mar 2024 10:08:12 +0100
> schrieb Angelo Dureghello <angelo@kernel-space.org>:
>
>> Add reset support for mcf5208.
>>
>> Signed-off-by: Angelo Dureghello <angelo@kernel-space.org>
>> ---
>> hw/m68k/mcf5208.c | 49 ++++++++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 46 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
>> index 0cfb806c20..d8a38274d0 100644
>> --- a/hw/m68k/mcf5208.c
>> +++ b/hw/m68k/mcf5208.c
>> @@ -40,6 +40,8 @@
>> #define PCSR_PRE_SHIFT 8
>> #define PCSR_PRE_MASK 0x0f00
>>
>> +#define RCR_SOFTRST 0x80
>> +
>> typedef struct {
>> MemoryRegion iomem;
>> qemu_irq irq;
>> @@ -185,13 +187,54 @@ static const MemoryRegionOps m5208_sys_ops = {
>> .endianness = DEVICE_NATIVE_ENDIAN,
>> };
>>
>> -static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic)
>> +static uint64_t m5208_rcm_read(void *opaque, hwaddr addr,
>> + unsigned size)
>> +{
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
>> + __func__, addr);
>
> I'd maybe rather not print this message if the guest tries to read from RCR
> or RSR - reading back a 0 should be fine here without logging a "bad
> offset".
>
>> + return 0;
>> +}
>> +
>> +static void m5208_rcm_write(void *opaque, hwaddr addr,
>> + uint64_t value, unsigned size)
>> +{
>> + M68kCPU *cpu = opaque;
>> + CPUState *cs = CPU(cpu);
>> + switch (addr) {
>> + case 0x0: /* RCR */
>> + if (value & RCR_SOFTRST) {
>> + cpu_reset(cs);
>> + cpu->env.aregs[7] = ldl_phys(cs->as, 0);
>> + cpu->env.pc = ldl_phys(cs->as, 4);
>> + }
>> + break;
>> + default:
>> + qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
>> + __func__, addr);
>> + break;
>> + }
>> +}
>> +
>> +static const MemoryRegionOps m5208_rcm_ops = {
>> + .read = m5208_rcm_read,
>> + .write = m5208_rcm_write,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic,
>> + M68kCPU *cpu)
>> {
>> - MemoryRegion *iomem = g_new(MemoryRegion, 1);
>> + MemoryRegion *iomem;
>
> I think it's more common coding style in the QEMU sources to have separate
> variables for separate MemoryRegions instead of re-using one variable for
> separate MemoryRegions like you do it below. See e.g.
> microchip_pfsoc_soc_realize() in hw/riscv/microchip_pfsoc.c.
>
>> m5208_timer_state *s;
>> int i;
>>
>> + /* RCM */
>> + iomem = g_new(MemoryRegion, 1);
>> + memory_region_init_io(iomem, NULL, &m5208_rcm_ops, cpu,
>> + "m5208-rcm", 0x00000080);
>> + memory_region_add_subregion(address_space, 0xfc0a0000, iomem);
>> /* SDRAMC. */
>> + iomem = g_new(MemoryRegion, 1);
>> memory_region_init_io(iomem, NULL, &m5208_sys_ops, NULL, "m5208-sys", 0x00004000);
>> memory_region_add_subregion(address_space, 0xfc0a8000, iomem);
>> /* Timers. */
>> @@ -265,7 +308,7 @@ static void mcf5208evb_init(MachineState *machine)
>> mcf_uart_create_mmap(0xfc064000, pic[27], serial_hd(1));
>> mcf_uart_create_mmap(0xfc068000, pic[28], serial_hd(2));
>>
>> - mcf5208_sys_init(address_space_mem, pic);
>> + mcf5208_sys_init(address_space_mem, pic, cpu);
>>
>> mcf_fec_init(address_space_mem, 0xfc030000, pic + 36);
>
> All in all, patch looks fine to me ... if you could update the above two
> nits by Monday, I can try to get this merged before the next soft freeze of
> QEMU (on Tuesday).
thanks a lot, sending v3.
>
> Thanks,
> Thomas
Regards,
angelo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-03-09 9:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-08 9:08 [PATCH v2] hw/m68k/mcf5208: add support for reset Angelo Dureghello
2024-03-09 9:06 ` Thomas Huth
2024-03-09 9:30 ` Angelo Dureghello
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).