Re: [PATCH 09/11] xen/arm: Setup MMIO range trap handlers for hardware domain

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

On 15.09.21 13:45, Rahul Singh wrote:
> Hi Oleksandr, Stefano,
>
>> On 15 Sep 2021, at 6:30 am, Oleksandr Andrushchenko 
>> <Oleksandr_Andrushchenko@xxxxxxxx> wrote:
>>
>> Hi, Rahul!
>>
>> On 14.09.21 17:24, Oleksandr Andrushchenko wrote:
>>> }
>>>>>    +static int pci_ecam_register_mmio_handler(struct domain *d,
>>>>> +                                          struct pci_host_bridge *bridge,
>>>>> +                                          const struct mmio_handler_ops 
>>>>> *ops)
>>>>> +{
>>>>> +    struct pci_config_window *cfg = bridge->sysdata;
>>>>> +
>>>>> +    register_mmio_handler(d, ops, cfg->phys_addr, cfg->size, NULL);
>>>>> +    return 0;
>>>>> +}
>>>> Given that struct pci_config_window is generic (it is not specific to
>>>> one bridge), I wonder if we even need the .register_mmio_handler
>>>> callback here.
>>>>
>>>> In fact,pci_host_bridge->sysdata doesn't even need to be a void*, it
>>>> could be a struct pci_config_window*, right?
>>> Rahul, this actually may change your series.
>>>
>>> Do you think we can do that?
>>>
>> This is the only change requested that left unanswered by now.
>>
>> Will it be possible that you change the API accordingly, so I can
>>
>> implement as Stefano suggests?
> We need pci_host_bridge->sysdata as void* in case we need to implement the 
> new non-ecam PCI controller in XEN.
> Please have a look once in Linux code[1] how bridge->sysdata will be used. 
> struct pci_config_window is used only for
> ecam supported host controller. Different PCI host controller will have 
> different PCI interface to access the PCI controller.
>
> [1] 
> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/pcie-rcar-host.c*n309__;Iw!!GF_29dbcQIUBPA!mbI_iuu-laYQoUn36kKf3z2H4AyxR4J8C59CcKb21pLldyVnDaKbgJSQhZ4liEnwnAe2uzK8OA$
>  [git[.]kernel[.]org]
>
> I think we need bridge->sysdata in future to implement the new PCI controller.
>
> Regards,
> Rahul
Stefano, does it sound reasonable then to keep the above code as is?
>   
>> Thanks,
>>
>> Oleksandr