Re: [RFC PATCH 2/2] plat/rcar: Add region id support for PCI

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

On 4/21/26 10:57, Mykyta Poturai wrote:

Hello Mykyta

> With Region ID enabled, all CPU memory accesses need to have rgid bits
> set in the physical address. This creates a problem for PCI BAR
> accesses, as it would require all BARs to be 64bit. Implement fixup_bar
> callback to add rgid bits to the address before mapping it to the
> guests.
For context. When Region ID support is enabled, the RGID value is 
encoded in the upper bits of physical addresses for both RAM and device 
MMIO in host device tree. From Xen's perspective, these are just regular 
physical addresses, and ideally Xen should not need to be RGID-aware at 
all. But I understand what this series is trying to solve.
It is worth mentioning that when Region ID support is enabled, there 
will be no 32-bit addresses in the host device tree anymore, since 
encoding the RGID effectively moves every address beyond the 4GB 
boundary (unless the RGID is 0). This might break any code in Xen that 
relies on 32-bit address space for some reason, e.g. 
HOST_ITS_WORKAROUND_32BIT_ADDR which requires 32-bit addresses for 
in-memory data structures (refer gicv3_its_enable_quirk_gen4()).

> Add Kconfig options to enable region id support and set the rgid value
> and physical address space size.
>
> Signed-off-by: Mykyta Poturai <mykyta_poturai@xxxxxxxx>
> ---
>   xen/arch/arm/pci/pci-host-rcar4.c | 15 +++++++++++++++
>   xen/arch/arm/platforms/Kconfig    | 21 +++++++++++++++++++++
>   2 files changed, 36 insertions(+)
>
> diff --git a/xen/arch/arm/pci/pci-host-rcar4.c 
> b/xen/arch/arm/pci/pci-host-rcar4.c
> index e1e8eb0ee1..50400c04c4 100644
> --- a/xen/arch/arm/pci/pci-host-rcar4.c
> +++ b/xen/arch/arm/pci/pci-host-rcar4.c
> @@ -43,6 +43,12 @@ struct rcar4_pcie_priv {
>       DECLARE_BITMAP(osid_regs, NUM_OSID_REGS);
>   };
>   
> +#define ULL(X) _AC(X, ULL)
> +#define MADDR_RGID(a)    (ULL(a) << CONFIG_RCAR_PA_BITS)
> +#define MADDR_PA_MASK    ((1ULL << CONFIG_RCAR_PA_BITS) - 1)
NIT: MADDR_PA_MASK is defined but never used ...

> +
> +#define MADDR_ENCODE_RGID(a)    (MADDR_RGID(CONFIG_RCAR_RGID) | (a))
> +
>   /*
>    * PCI host bridges often have different ways to access the root and child
>    * bus config spaces:
> @@ -61,6 +67,14 @@ static int __init rcar4_child_cfg_reg_index(struct 
> dt_device_node *np)
>       return dt_property_match_string(np, "reg-names", "config");
>   }
>   
> +static void rcar4_pcie_fixup_bar(struct pci_host_bridge *bridge,
> +                                 unsigned int bar_num,
> +                                 paddr_t *addr)
> +{
> +    if ( IS_ENABLED(CONFIG_RCAR_REGION_ID_SUPPORT) )
> +        *addr = MADDR_ENCODE_RGID(*addr);
> +}
> +
>   /* ECAM ops */
>   static const struct pci_ecam_ops rcar4_pcie_ops = {
>       .bus_shift  = 20,
> @@ -71,6 +85,7 @@ static const struct pci_ecam_ops rcar4_pcie_ops = {
>           .write                  = pci_generic_config_write,
>           .need_p2m_hwdom_mapping = pci_ecam_need_p2m_hwdom_mapping,
>           .init_bus_range         = pci_generic_init_bus_range,
> +        .fixup_bar              = rcar4_pcie_fixup_bar,
>       }
>   };
>   
> diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig
> index 888d0b85d5..db096952c8 100644
> --- a/xen/arch/arm/platforms/Kconfig
> +++ b/xen/arch/arm/platforms/Kconfig
> @@ -64,6 +64,27 @@ config NO_PLAT
>   
>   endchoice
>   
> +menu "RCar Region ID Support"
> +    visible if RCAR4
> +
> +config RCAR_REGION_ID_SUPPORT
> +       bool "Renesas Region ID support for R-Car Gen4 platforms" if EXPERT
> +       depends on RCAR4
> +       help
> +         Enable experimental Region ID support for R-Car Gen4 platforms
> +
> +config RCAR_RGID
> +       int "Region ID encoded in physical address"
> +       depends on RCAR_REGION_ID_SUPPORT
> +       default 0
> +
> +config RCAR_PA_BITS
> +       int "Physical address space size"
> +       depends on RCAR_REGION_ID_SUPPORT
> +       default 36
Options RCAR_RGID and RCAR_PA_BITS lack range constraints. A 
misconfigured options might cause undefined shift behavior at compile 
time. However, I see a valid request in a separate email to consider 
using a firmware table to pass these values, so just ignore this comment.
*****

I was wondering whether the RGID could be extracted at runtime from an
address that already carries it. Do addresses in the host device tree 
(e.g., device MMIO ranges, host bridge windows, etc) already contain 
RGID bits? If so, then in rcar4_pcie_fixup_bar() you could obtain the 
RGID from the bridge instance that is already passed as a parameter.
Something like below:

#define MADDR_GET_RGID(addr)  ((addr) >> CONFIG_RCAR_PA_BITS)

paddr_t rgid = 
MADDR_GET_RGID(bridge->some_hw_addr_as_it_is_specified_in_dt);
*addr = (rgid << CONFIG_RCAR_PA_BITS) | (*addr & MADDR_PA_MASK);

This would eliminate CONFIG_RCAR_RGID entirely by deducing it from
existing platform data rather than requiring it to be passed explicitly
via the device tree.

As for CONFIG_RCAR_PA_BITS, I think it could remain as-is since it is
constant across an SoC generation, unless I am mistaken. As far as I am 
aware, on Gen4 the RGID occupies 4 bits (0–15) starting at bit 36, and 
on Gen5 it occupies 5 bits (0–31) starting at bit 43.
Since Xen can identify the hardware it is running on (thanks to the 
compatible string), the appropriate #define-s could be applied.
Would the above approach work, or is it better to pass all these values 
explicitly?
Please note, I am not 100% sure whether this is the right and reliable 
approach, but it should be considered.


> +
> +endmenu
> +
>   config ALL64_PLAT
>         bool
>         default (ALL_PLAT && ARM_64)


F