On 2/18/19 10:33 AM, Sven Schnelle wrote: > some versions of HP-UX 10.20 seems to rely on the fact that DINO > strips out the lower 2 bits of the PCI configuration address. > Also update the binary SeaBIOS distributed to the latest version > from Helge's repository, which is required with that change. > > Signed-off-by: Sven Schnelle <sv...@stackframe.org> > --- > hw/hppa/dino.c | 25 ++++++++++++++++++++++--- > pc-bios/hppa-firmware.img | Bin 215936 -> 760040 bytes > 2 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c > index 360716de57..63441783c4 100644 > --- a/hw/hppa/dino.c > +++ b/hw/hppa/dino.c > @@ -178,7 +178,7 @@ static MemTxResult dino_chip_read_with_attrs(void > *opaque, hwaddr addr, > case DINO_PCI_IO_DATA ... DINO_PCI_IO_DATA + 3: > /* Read from PCI IO space. */ > io = &address_space_io; > - ioaddr = s->parent_obj.config_reg; > + ioaddr = s->parent_obj.config_reg + (addr & 3); > switch (size) { > case 1: > val = address_space_ldub(io, ioaddr, attrs, &ret); > @@ -250,7 +250,7 @@ static MemTxResult dino_chip_write_with_attrs(void > *opaque, hwaddr addr, > case DINO_IO_DATA ... DINO_PCI_IO_DATA + 3: > /* Write into PCI IO space. */ > io = &address_space_io; > - ioaddr = s->parent_obj.config_reg; > + ioaddr = s->parent_obj.config_reg + (addr & 3); > switch (size) { > case 1: > address_space_stb(io, ioaddr, val, attrs, &ret); > @@ -360,6 +360,25 @@ static const MemoryRegionOps dino_config_data_ops = { > .endianness = DEVICE_LITTLE_ENDIAN, > }; > > +static uint64_t dino_config_addr_read(void *opaque, hwaddr addr, unsigned > len) > +{ > + PCIHostState *s = opaque; > + return s->config_reg & ~3U; > +} > + > +static void dino_config_addr_write(void *opaque, hwaddr addr, > + uint64_t val, unsigned len) > +{ > + PCIHostState *s = opaque; > + s->config_reg = val & ~3U; > +} > + > +static const MemoryRegionOps dino_config_addr_ops = { > + .read = dino_config_addr_read, > + .write = dino_config_addr_write, > + .endianness = DEVICE_BIG_ENDIAN, > +}; > +
Can you re-test with the following? If the stores are all aligned, then the reads to not need to re-align. Also, partial writes to the config register don't make much sense. Over in hw/pci/pci_host.c, we check addr == 0 && len == 4, which is a very old fashioned way to do the .valid thing. r~ diff --git a/hw/hppa/dino.c b/hw/hppa/dino.c index 63441783c4..40f9e1a963 100644 --- a/hw/hppa/dino.c +++ b/hw/hppa/dino.c @@ -363,7 +363,7 @@ static const MemoryRegionOps dino_config_data_ops = { static uint64_t dino_config_addr_read(void *opaque, hwaddr addr, unsigned len) { PCIHostState *s = opaque; - return s->config_reg & ~3U; + return s->config_reg; } static void dino_config_addr_write(void *opaque, hwaddr addr, @@ -376,6 +376,8 @@ static void dino_config_addr_write(void *opaque, hwaddr addr, static const MemoryRegionOps dino_config_addr_ops = { .read = dino_config_addr_read, .write = dino_config_addr_write, + .valid.min_access_size = 4, + .valid.max_access_size = 4, .endianness = DEVICE_BIG_ENDIAN, };