On Thursday 10 November 2022 12:50:13 Jonathan Derrick wrote:
> Add a handler to set behavior of a PCI or PCIe register. Add the
> appropriate enums to specify the register's Read-Only, Read-Write, and
> Write-1-to-Clear behaviors.
>
> Signed-off-by: Jonathan Derrick <jonathan.derrick@xxxxxxxxx>
I do not think that this is the correct way. Drivers should not need to
tell bridge emulator that some register is read-only or read-write.
Bridge emulator has already all required information in its internal
structures.
If there is a need to tell bridge emulator to "emulate" some optional
feature, for example hot plug capabilities, then it would be better to
extend pci_bridge_emul_init() flags and let init function to correctly
fill all behavior bits. There is already PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD
flag which modify bridge prefetchable bits.
In my opinion, all standard PCI/PCIe behavior bits should be in
pci-bridge-emul.c source file and drivers should not modify them.
I think that this approach makes implementation lot of cleaner.
> ---
> drivers/pci/pci-bridge-emul.c | 19 +++++++++++++++++++
> drivers/pci/pci-bridge-emul.h | 10 ++++++++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c
> index 9334b2dd4764..3c1a683ece66 100644
> --- a/drivers/pci/pci-bridge-emul.c
> +++ b/drivers/pci/pci-bridge-emul.c
> @@ -46,6 +46,25 @@ struct pci_bridge_reg_behavior {
> u32 w1c;
> };
>
> +void pci_bridge_emul_set_reg_behavior(struct pci_bridge_emul *bridge,
> + bool pcie, int reg, u32 val,
> + enum pci_bridge_emul_reg_behavior type)
> +{
> + struct pci_bridge_reg_behavior *behavior;
> +
> + if (pcie)
> + behavior = &bridge->pcie_cap_regs_behavior[reg / 4];
> + else
> + behavior = &bridge->pci_regs_behavior[reg / 4];
> +
> + if (type == PCI_BRIDGE_EMUL_REG_BEHAVIOR_RO)
> + behavior->ro = val;
> + else if (type == PCI_BRIDGE_EMUL_REG_BEHAVIOR_RW)
> + behavior->rw = val;
> + else /* PCI_BRIDGE_EMUL_REG_BEHAVIOR_W1C */
> + behavior->w1c = val;
> +}
> +
> static const
> struct pci_bridge_reg_behavior pci_regs_behavior[PCI_STD_HEADER_SIZEOF / 4] = {
> [PCI_VENDOR_ID / 4] = { .ro = ~0 },
> diff --git a/drivers/pci/pci-bridge-emul.h b/drivers/pci/pci-bridge-emul.h
> index 2a0e59c7f0d9..b2401d58518c 100644
> --- a/drivers/pci/pci-bridge-emul.h
> +++ b/drivers/pci/pci-bridge-emul.h
> @@ -72,6 +72,12 @@ struct pci_bridge_emul;
> typedef enum { PCI_BRIDGE_EMUL_HANDLED,
> PCI_BRIDGE_EMUL_NOT_HANDLED } pci_bridge_emul_read_status_t;
>
> +enum pci_bridge_emul_reg_behavior {
> + PCI_BRIDGE_EMUL_REG_BEHAVIOR_RO,
> + PCI_BRIDGE_EMUL_REG_BEHAVIOR_RW,
> + PCI_BRIDGE_EMUL_REG_BEHAVIOR_W1C,
> +};
> +
> struct pci_bridge_emul_ops {
> /*
> * Called when reading from the regular PCI bridge
> @@ -161,4 +167,8 @@ int pci_bridge_emul_conf_read(struct pci_bridge_emul *bridge, int where,
> int pci_bridge_emul_conf_write(struct pci_bridge_emul *bridge, int where,
> int size, u32 value);
>
> +void pci_bridge_emul_set_reg_behavior(struct pci_bridge_emul *bridge,
> + bool pcie, int reg, u32 val,
> + enum pci_bridge_emul_reg_behavior type);
> +
> #endif /* __PCI_BRIDGE_EMUL_H__ */
> --
> 2.30.2
>
