| Message ID | 20230119222845.27209745706@zero.eik.bme.hu |
|---|---|
| State | New |
| Headers | show |
| Series | mac_nvram: Add block backend to persist NVRAM contents | expand |
On 19/1/23 23:28, BALATON Zoltan wrote: > Add a way to set a backing store for the mac_nvram similar to what > spapr_nvram or mac_via PRAM already does to allow to save its contents > between runs. Use -drive file=nvram.img,format=raw,if=mtd to specify > backing file where nvram.img must be MACIO_NVRAM_SIZE which is 8192 > bytes. It is only wired for mac_oldworld for now but could be used by > mac_newworld in the future too. > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/nvram/mac_nvram.c | 28 ++++++++++++++++++++++++++++ > hw/ppc/mac_oldworld.c | 8 +++++++- > include/hw/nvram/mac_nvram.h | 1 + > 3 files changed, 36 insertions(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 1/19/23 23:28, BALATON Zoltan wrote: > Add a way to set a backing store for the mac_nvram similar to what > spapr_nvram or mac_via PRAM already does to allow to save its contents > between runs. Use -drive file=nvram.img,format=raw,if=mtd to specify > backing file where nvram.img must be MACIO_NVRAM_SIZE which is 8192 > bytes. It is only wired for mac_oldworld for now but could be used by > mac_newworld in the future too. > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/nvram/mac_nvram.c | 28 ++++++++++++++++++++++++++++ > hw/ppc/mac_oldworld.c | 8 +++++++- > include/hw/nvram/mac_nvram.h | 1 + > 3 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/hw/nvram/mac_nvram.c b/hw/nvram/mac_nvram.c > index 3d9ddda217..810e84f07e 100644 > --- a/hw/nvram/mac_nvram.c > +++ b/hw/nvram/mac_nvram.c > @@ -24,9 +24,12 @@ > */ > > #include "qemu/osdep.h" > +#include "qapi/error.h" > #include "hw/nvram/chrp_nvram.h" > #include "hw/nvram/mac_nvram.h" > #include "hw/qdev-properties.h" > +#include "hw/qdev-properties-system.h" > +#include "sysemu/block-backend.h" > #include "migration/vmstate.h" > #include "qemu/cutils.h" > #include "qemu/module.h" > @@ -44,6 +47,9 @@ static void macio_nvram_writeb(void *opaque, hwaddr addr, > addr = (addr >> s->it_shift) & (s->size - 1); > trace_macio_nvram_write(addr, value); > s->data[addr] = value; > + if (s->blk) { > + blk_pwrite(s->blk, addr, 1, &s->data[addr], 0); > + } > } > > static uint64_t macio_nvram_readb(void *opaque, hwaddr addr, > @@ -91,6 +97,27 @@ static void macio_nvram_realizefn(DeviceState *dev, Error **errp) > > s->data = g_malloc0(s->size); > > + if (s->blk) { > + int64_t len = blk_getlength(s->blk); > + if (len < 0) { > + error_setg_errno(errp, -len, > + "could not get length of nvram backing image"); > + return; > + } else if (len != s->size) { > + error_setg_errno(errp, -len, > + "invalid size nvram backing image"); > + return; > + } > + if (blk_set_perm(s->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE, > + BLK_PERM_ALL, errp) < 0) { > + return; > + } > + if (blk_pread(s->blk, 0, s->size, s->data, 0) < 0) { > + error_setg(errp, "can't read-nvram contents"); > + return; > + } This could use blk_check_size_and_read_all() instead ? C. > + } > + > memory_region_init_io(&s->mem, OBJECT(s), &macio_nvram_ops, s, > "macio-nvram", s->size << s->it_shift); > sysbus_init_mmio(d, &s->mem); > @@ -106,6 +133,7 @@ static void macio_nvram_unrealizefn(DeviceState *dev) > static Property macio_nvram_properties[] = { > DEFINE_PROP_UINT32("size", MacIONVRAMState, size, 0), > DEFINE_PROP_UINT32("it_shift", MacIONVRAMState, it_shift, 0), > + DEFINE_PROP_DRIVE("drive", MacIONVRAMState, blk), > DEFINE_PROP_END_OF_LIST() > }; > > diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c > index e052ad880e..52e554710f 100644 > --- a/hw/ppc/mac_oldworld.c > +++ b/hw/ppc/mac_oldworld.c > @@ -103,7 +103,7 @@ static void ppc_heathrow_init(MachineState *machine) > DeviceState *dev, *pic_dev, *grackle_dev; > BusState *adb_bus; > uint16_t ppc_boot_device; > - DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; > + DriveInfo *dinfo, *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; > void *fw_cfg; > uint64_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TBFREQ; > uint8_t *spd_data[3] = {}; > @@ -256,6 +256,12 @@ static void ppc_heathrow_init(MachineState *machine) > qdev_prop_set_chr(dev, "chrA", serial_hd(0)); > qdev_prop_set_chr(dev, "chrB", serial_hd(1)); > > + dinfo = drive_get(IF_MTD, 0, 0); > + if (dinfo) { > + dev = DEVICE(object_resolve_path_component(macio, "nvram")); > + qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo)); > + } > + > pci_realize_and_unref(PCI_DEVICE(macio), pci_bus, &error_fatal); > > pic_dev = DEVICE(object_resolve_path_component(macio, "pic")); > diff --git a/include/hw/nvram/mac_nvram.h b/include/hw/nvram/mac_nvram.h > index b780aca470..0c4dfaeff6 100644 > --- a/include/hw/nvram/mac_nvram.h > +++ b/include/hw/nvram/mac_nvram.h > @@ -44,6 +44,7 @@ struct MacIONVRAMState { > > MemoryRegion mem; > uint8_t *data; > + BlockBackend *blk; > }; > > void pmac_format_nvram_partition(MacIONVRAMState *nvr, int len);
On Fri, 20 Jan 2023, Cédric Le Goater wrote: > On 1/19/23 23:28, BALATON Zoltan wrote: >> Add a way to set a backing store for the mac_nvram similar to what >> spapr_nvram or mac_via PRAM already does to allow to save its contents >> between runs. Use -drive file=nvram.img,format=raw,if=mtd to specify >> backing file where nvram.img must be MACIO_NVRAM_SIZE which is 8192 >> bytes. It is only wired for mac_oldworld for now but could be used by >> mac_newworld in the future too. >> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> >> --- >> hw/nvram/mac_nvram.c | 28 ++++++++++++++++++++++++++++ >> hw/ppc/mac_oldworld.c | 8 +++++++- >> include/hw/nvram/mac_nvram.h | 1 + >> 3 files changed, 36 insertions(+), 1 deletion(-) >> >> diff --git a/hw/nvram/mac_nvram.c b/hw/nvram/mac_nvram.c >> index 3d9ddda217..810e84f07e 100644 >> --- a/hw/nvram/mac_nvram.c >> +++ b/hw/nvram/mac_nvram.c >> @@ -24,9 +24,12 @@ >> */ >> #include "qemu/osdep.h" >> +#include "qapi/error.h" >> #include "hw/nvram/chrp_nvram.h" >> #include "hw/nvram/mac_nvram.h" >> #include "hw/qdev-properties.h" >> +#include "hw/qdev-properties-system.h" >> +#include "sysemu/block-backend.h" >> #include "migration/vmstate.h" >> #include "qemu/cutils.h" >> #include "qemu/module.h" >> @@ -44,6 +47,9 @@ static void macio_nvram_writeb(void *opaque, hwaddr addr, >> addr = (addr >> s->it_shift) & (s->size - 1); >> trace_macio_nvram_write(addr, value); >> s->data[addr] = value; >> + if (s->blk) { >> + blk_pwrite(s->blk, addr, 1, &s->data[addr], 0); >> + } >> } >> static uint64_t macio_nvram_readb(void *opaque, hwaddr addr, >> @@ -91,6 +97,27 @@ static void macio_nvram_realizefn(DeviceState *dev, >> Error **errp) >> s->data = g_malloc0(s->size); >> + if (s->blk) { >> + int64_t len = blk_getlength(s->blk); >> + if (len < 0) { >> + error_setg_errno(errp, -len, >> + "could not get length of nvram backing >> image"); >> + return; >> + } else if (len != s->size) { >> + error_setg_errno(errp, -len, >> + "invalid size nvram backing image"); >> + return; >> + } >> + if (blk_set_perm(s->blk, BLK_PERM_CONSISTENT_READ | >> BLK_PERM_WRITE, >> + BLK_PERM_ALL, errp) < 0) { >> + return; >> + } >> + if (blk_pread(s->blk, 0, s->size, s->data, 0) < 0) { >> + error_setg(errp, "can't read-nvram contents"); >> + return; >> + } > > This could use blk_check_size_and_read_all() instead ? Good idea, except that comments in that function say its error handling is not very good and tends to give unuseful messages to users so maybe until that's sorted out I'd stay with open coding it here. Regards, BALATON Zoltan
On 19/01/2023 22:28, BALATON Zoltan wrote: > Add a way to set a backing store for the mac_nvram similar to what > spapr_nvram or mac_via PRAM already does to allow to save its contents > between runs. Use -drive file=nvram.img,format=raw,if=mtd to specify > backing file where nvram.img must be MACIO_NVRAM_SIZE which is 8192 > bytes. It is only wired for mac_oldworld for now but could be used by > mac_newworld in the future too. > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/nvram/mac_nvram.c | 28 ++++++++++++++++++++++++++++ > hw/ppc/mac_oldworld.c | 8 +++++++- > include/hw/nvram/mac_nvram.h | 1 + > 3 files changed, 36 insertions(+), 1 deletion(-) > > diff --git a/hw/nvram/mac_nvram.c b/hw/nvram/mac_nvram.c > index 3d9ddda217..810e84f07e 100644 > --- a/hw/nvram/mac_nvram.c > +++ b/hw/nvram/mac_nvram.c > @@ -24,9 +24,12 @@ > */ > > #include "qemu/osdep.h" > +#include "qapi/error.h" > #include "hw/nvram/chrp_nvram.h" > #include "hw/nvram/mac_nvram.h" > #include "hw/qdev-properties.h" > +#include "hw/qdev-properties-system.h" > +#include "sysemu/block-backend.h" > #include "migration/vmstate.h" > #include "qemu/cutils.h" > #include "qemu/module.h" > @@ -44,6 +47,9 @@ static void macio_nvram_writeb(void *opaque, hwaddr addr, > addr = (addr >> s->it_shift) & (s->size - 1); > trace_macio_nvram_write(addr, value); > s->data[addr] = value; > + if (s->blk) { > + blk_pwrite(s->blk, addr, 1, &s->data[addr], 0); > + } > } > > static uint64_t macio_nvram_readb(void *opaque, hwaddr addr, > @@ -91,6 +97,27 @@ static void macio_nvram_realizefn(DeviceState *dev, Error **errp) > > s->data = g_malloc0(s->size); > > + if (s->blk) { > + int64_t len = blk_getlength(s->blk); > + if (len < 0) { > + error_setg_errno(errp, -len, > + "could not get length of nvram backing image"); > + return; > + } else if (len != s->size) { > + error_setg_errno(errp, -len, > + "invalid size nvram backing image"); > + return; > + } > + if (blk_set_perm(s->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE, > + BLK_PERM_ALL, errp) < 0) { > + return; > + } > + if (blk_pread(s->blk, 0, s->size, s->data, 0) < 0) { > + error_setg(errp, "can't read-nvram contents"); > + return; > + } > + } > + > memory_region_init_io(&s->mem, OBJECT(s), &macio_nvram_ops, s, > "macio-nvram", s->size << s->it_shift); > sysbus_init_mmio(d, &s->mem); > @@ -106,6 +133,7 @@ static void macio_nvram_unrealizefn(DeviceState *dev) > static Property macio_nvram_properties[] = { > DEFINE_PROP_UINT32("size", MacIONVRAMState, size, 0), > DEFINE_PROP_UINT32("it_shift", MacIONVRAMState, it_shift, 0), > + DEFINE_PROP_DRIVE("drive", MacIONVRAMState, blk), > DEFINE_PROP_END_OF_LIST() > }; > > diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c > index e052ad880e..52e554710f 100644 > --- a/hw/ppc/mac_oldworld.c > +++ b/hw/ppc/mac_oldworld.c > @@ -103,7 +103,7 @@ static void ppc_heathrow_init(MachineState *machine) > DeviceState *dev, *pic_dev, *grackle_dev; > BusState *adb_bus; > uint16_t ppc_boot_device; > - DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; > + DriveInfo *dinfo, *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; > void *fw_cfg; > uint64_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TBFREQ; > uint8_t *spd_data[3] = {}; > @@ -256,6 +256,12 @@ static void ppc_heathrow_init(MachineState *machine) > qdev_prop_set_chr(dev, "chrA", serial_hd(0)); > qdev_prop_set_chr(dev, "chrB", serial_hd(1)); > > + dinfo = drive_get(IF_MTD, 0, 0); > + if (dinfo) { > + dev = DEVICE(object_resolve_path_component(macio, "nvram")); > + qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo)); > + } > + > pci_realize_and_unref(PCI_DEVICE(macio), pci_bus, &error_fatal); > > pic_dev = DEVICE(object_resolve_path_component(macio, "pic")); > diff --git a/include/hw/nvram/mac_nvram.h b/include/hw/nvram/mac_nvram.h > index b780aca470..0c4dfaeff6 100644 > --- a/include/hw/nvram/mac_nvram.h > +++ b/include/hw/nvram/mac_nvram.h > @@ -44,6 +44,7 @@ struct MacIONVRAMState { > > MemoryRegion mem; > uint8_t *data; > + BlockBackend *blk; > }; > > void pmac_format_nvram_partition(MacIONVRAMState *nvr, int len); This looks okay as far as I can tell, however you want to split this into 2 patches: the first which makes the changes to the mac_nvram device, and the second to wire it up to the g3beige machine. ATB, Mark.
diff --git a/hw/nvram/mac_nvram.c b/hw/nvram/mac_nvram.c index 3d9ddda217..810e84f07e 100644 --- a/hw/nvram/mac_nvram.c +++ b/hw/nvram/mac_nvram.c @@ -24,9 +24,12 @@ */ #include "qemu/osdep.h" +#include "qapi/error.h" #include "hw/nvram/chrp_nvram.h" #include "hw/nvram/mac_nvram.h" #include "hw/qdev-properties.h" +#include "hw/qdev-properties-system.h" +#include "sysemu/block-backend.h" #include "migration/vmstate.h" #include "qemu/cutils.h" #include "qemu/module.h" @@ -44,6 +47,9 @@ static void macio_nvram_writeb(void *opaque, hwaddr addr, addr = (addr >> s->it_shift) & (s->size - 1); trace_macio_nvram_write(addr, value); s->data[addr] = value; + if (s->blk) { + blk_pwrite(s->blk, addr, 1, &s->data[addr], 0); + } } static uint64_t macio_nvram_readb(void *opaque, hwaddr addr, @@ -91,6 +97,27 @@ static void macio_nvram_realizefn(DeviceState *dev, Error **errp) s->data = g_malloc0(s->size); + if (s->blk) { + int64_t len = blk_getlength(s->blk); + if (len < 0) { + error_setg_errno(errp, -len, + "could not get length of nvram backing image"); + return; + } else if (len != s->size) { + error_setg_errno(errp, -len, + "invalid size nvram backing image"); + return; + } + if (blk_set_perm(s->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE, + BLK_PERM_ALL, errp) < 0) { + return; + } + if (blk_pread(s->blk, 0, s->size, s->data, 0) < 0) { + error_setg(errp, "can't read-nvram contents"); + return; + } + } + memory_region_init_io(&s->mem, OBJECT(s), &macio_nvram_ops, s, "macio-nvram", s->size << s->it_shift); sysbus_init_mmio(d, &s->mem); @@ -106,6 +133,7 @@ static void macio_nvram_unrealizefn(DeviceState *dev) static Property macio_nvram_properties[] = { DEFINE_PROP_UINT32("size", MacIONVRAMState, size, 0), DEFINE_PROP_UINT32("it_shift", MacIONVRAMState, it_shift, 0), + DEFINE_PROP_DRIVE("drive", MacIONVRAMState, blk), DEFINE_PROP_END_OF_LIST() }; diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c index e052ad880e..52e554710f 100644 --- a/hw/ppc/mac_oldworld.c +++ b/hw/ppc/mac_oldworld.c @@ -103,7 +103,7 @@ static void ppc_heathrow_init(MachineState *machine) DeviceState *dev, *pic_dev, *grackle_dev; BusState *adb_bus; uint16_t ppc_boot_device; - DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; + DriveInfo *dinfo, *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; void *fw_cfg; uint64_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TBFREQ; uint8_t *spd_data[3] = {}; @@ -256,6 +256,12 @@ static void ppc_heathrow_init(MachineState *machine) qdev_prop_set_chr(dev, "chrA", serial_hd(0)); qdev_prop_set_chr(dev, "chrB", serial_hd(1)); + dinfo = drive_get(IF_MTD, 0, 0); + if (dinfo) { + dev = DEVICE(object_resolve_path_component(macio, "nvram")); + qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo)); + } + pci_realize_and_unref(PCI_DEVICE(macio), pci_bus, &error_fatal); pic_dev = DEVICE(object_resolve_path_component(macio, "pic")); diff --git a/include/hw/nvram/mac_nvram.h b/include/hw/nvram/mac_nvram.h index b780aca470..0c4dfaeff6 100644 --- a/include/hw/nvram/mac_nvram.h +++ b/include/hw/nvram/mac_nvram.h @@ -44,6 +44,7 @@ struct MacIONVRAMState { MemoryRegion mem; uint8_t *data; + BlockBackend *blk; }; void pmac_format_nvram_partition(MacIONVRAMState *nvr, int len);
Add a way to set a backing store for the mac_nvram similar to what spapr_nvram or mac_via PRAM already does to allow to save its contents between runs. Use -drive file=nvram.img,format=raw,if=mtd to specify backing file where nvram.img must be MACIO_NVRAM_SIZE which is 8192 bytes. It is only wired for mac_oldworld for now but could be used by mac_newworld in the future too. Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/nvram/mac_nvram.c | 28 ++++++++++++++++++++++++++++ hw/ppc/mac_oldworld.c | 8 +++++++- include/hw/nvram/mac_nvram.h | 1 + 3 files changed, 36 insertions(+), 1 deletion(-)