On Mon, 27 Oct 2025 at 16:54, Philippe Mathieu-Daudé <[email protected]> wrote:
>
> In order to have more PCI host bridges to re-use the
> generic pci_host_data_le_ops MemoryRegionOps, add the
> 'config-reg-check-high-bit' property (%true by default).
>
> Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
> diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
> index b5c624e12e8..d6db365e327 100644
> --- a/hw/pci/pci_host.c
> +++ b/hw/pci/pci_host.c
> @@ -184,8 +184,10 @@ static void pci_host_data_write(void *opaque, hwaddr
> addr,
> {
> PCIHostState *s = opaque;
>
> - if (s->config_reg & (1u << 31))
> - pci_data_write(s->bus, s->config_reg | (addr & 3), val, len);
> + if (s->config_reg_check_high_bit && !(s->config_reg & (1U << 31))) {
> + return;
> + }
> + pci_data_write(s->bus, s->config_reg | (addr & 3), val, len);
> }
>
> static uint64_t pci_host_data_read(void *opaque,
> @@ -193,7 +195,7 @@ static uint64_t pci_host_data_read(void *opaque,
> {
> PCIHostState *s = opaque;
>
> - if (!(s->config_reg & (1U << 31))) {
> + if (s->config_reg_check_high_bit && !(s->config_reg & (1U << 31))) {
> return 0xffffffff;
> }
> return pci_data_read(s->bus, s->config_reg | (addr & 3), len);
> @@ -235,6 +237,8 @@ const VMStateDescription vmstate_pcihost = {
> };
>
> static const Property pci_host_properties_common[] = {
> + DEFINE_PROP_BOOL("config-reg-check-high-bit", PCIHostState,
> + config_reg_check_high_bit, true),
I think it might be useful to name and document this
property at a slightly higher level of abstraction.
Specifically, this code is handling the behaviour of
the CONFIG_ADDRESS register which is part of the PCI
Configuration Access Method (CAM). For x86 the top bit of
CONFIG_ADDRESS is an Enable bit, which must be set to
cause accesses to CONFIG_DATA to actually do something.
For PCI controllers like Dino there is no Enable bit
defined in CONFIG_ADDRESS[*] and CONFIG_DATA accesses always
take effect.
[*] http://ftp.parisc-linux.org/docs/chips/dino_ers.pdf page 49
So perhaps we could call this "config-address-reg-has-enable-bit" ?
A documentation comment about its purpose and noting that
the expectation is that this is set by the subclass,
not by end-users, might also be helpful.
thanks
-- PMM
