Re: [PATCH v8 2/4] xen/arm: setup MMIO range trap handlers for hardware domain
On 10/12/2021 18:37, Oleksandr Andrushchenko wrote:
Hi, Julien!
Hello,
On 10.12.21 19:52, Julien Grall wrote:
Hi Oleksandr,
On 09/12/2021 07:29, Oleksandr Andrushchenko wrote:
+unsigned int domain_vpci_get_num_mmio_handlers(struct domain *d)
+{
+ if ( !has_vpci(d) )
+ return 0;
+
+ if ( is_hardware_domain(d) )
+ {
+ int ret = pci_host_iterate_bridges_and_count(d,
vpci_get_num_handlers_cb);
+
+ return ret < 0 ? 0 : ret;
Sorry I only spotted this now. AFAICT, ret is not meant to return ret < 0 in
this case. But if it were then I think it would be wrong to continue as nothing
happened because the code will likely fall over/crash when registering the I/O
handlers.
I would document this oddity with
if ( ret < 0 )
{
ASSERT_UNREACHABLE();
return 0;
}
I can do the change on commit if the others are happy with it.
Yes, please, do me a favor
Ok. With that:
Acked-by: Julien Grall
Cheers,
Cheers,
Thank you,
Oleksandr
--
Julien Grall
Re: [PATCH v8 2/4] xen/arm: setup MMIO range trap handlers for hardware domain
Hi, Julien!
On 10.12.21 19:52, Julien Grall wrote:
> Hi Oleksandr,
>
> On 09/12/2021 07:29, Oleksandr Andrushchenko wrote:
>> +unsigned int domain_vpci_get_num_mmio_handlers(struct domain *d)
>> +{
>> + if ( !has_vpci(d) )
>> + return 0;
>> +
>> + if ( is_hardware_domain(d) )
>> + {
>> + int ret = pci_host_iterate_bridges_and_count(d,
>> vpci_get_num_handlers_cb);
>> +
>> + return ret < 0 ? 0 : ret;
>
> Sorry I only spotted this now. AFAICT, ret is not meant to return ret < 0 in
> this case. But if it were then I think it would be wrong to continue as
> nothing happened because the code will likely fall over/crash when
> registering the I/O handlers.
>
> I would document this oddity with
>
> if ( ret < 0 )
> {
> ASSERT_UNREACHABLE();
> return 0;
> }
>
> I can do the change on commit if the others are happy with it.
Yes, please, do me a favor
>
> Cheers,
>
Thank you,
Oleksandr
Re: [PATCH v8 2/4] xen/arm: setup MMIO range trap handlers for hardware domain
Hi Oleksandr,
On 09/12/2021 07:29, Oleksandr Andrushchenko wrote:
+unsigned int domain_vpci_get_num_mmio_handlers(struct domain *d)
+{
+if ( !has_vpci(d) )
+return 0;
+
+if ( is_hardware_domain(d) )
+{
+int ret = pci_host_iterate_bridges_and_count(d,
vpci_get_num_handlers_cb);
+
+return ret < 0 ? 0 : ret;
Sorry I only spotted this now. AFAICT, ret is not meant to return ret <
0 in this case. But if it were then I think it would be wrong to
continue as nothing happened because the code will likely fall
over/crash when registering the I/O handlers.
I would document this oddity with
if ( ret < 0 )
{
ASSERT_UNREACHABLE();
return 0;
}
I can do the change on commit if the others are happy with it.
Cheers,
--
Julien Grall
Re: [PATCH v8 2/4] xen/arm: setup MMIO range trap handlers for hardware domain
Hi Oleksandr, > On 9 Dec 2021, at 7:29 am, Oleksandr Andrushchenko wrote: > > From: Oleksandr Andrushchenko > > In order for vPCI to work it needs to maintain guest and hardware > domain's views of the configuration space. For example, BARs and > COMMAND registers require emulation for guests and the guest view > of the registers needs to be in sync with the real contents of the > relevant registers. For that ECAM address space needs to also be > trapped for the hardware domain, so we need to implement PCI host > bridge specific callbacks to properly setup MMIO handlers for those > ranges depending on particular host bridge implementation. > > Signed-off-by: Oleksandr Andrushchenko Reviewed-by: Rahul Singh Tested-by: Rahul Singh Regards, Rahul
