On 21.03.2025 15:50, Mykyta Poturai wrote: > On 21.03.25 15:41, Jan Beulich wrote: >> On 21.03.2025 11:56, Mykyta Poturai wrote: >>> On 17.03.25 17:07, Jan Beulich wrote: >>>> On 14.03.2025 14:34, Mykyta Poturai wrote: >>>>> --- a/xen/drivers/pci/physdev.c >>>>> +++ b/xen/drivers/pci/physdev.c >>>>> @@ -19,7 +19,7 @@ ret_t pci_physdev_op(int cmd, >>>>> XEN_GUEST_HANDLE_PARAM(void) arg) >>>>> struct pci_dev_info pdev_info; >>>>> nodeid_t node = NUMA_NO_NODE; >>>>> >>>>> - if ( !is_pci_passthrough_enabled() ) >>>>> + if ( !is_pci_passthrough_enabled(true) ) >>>>> return -EOPNOTSUPP; >>>> >>>> Seeing the function's parameter name, how do you know it's Dom0 calling >>>> here? >>> >>> Is this a functional or naming concern? If it is about naming then can >>> it also be solved by renaming the parameter? >> >> The renaming suggested above would resolve this, yes. Whether "for_pci_hwdom" >> or anything alike is a good parameter name is a different question. >> >>> Regarding functional issues, I have assumed that only hwdom can make >>> physdev operations, but after checking it, this assumption seems to be >>> correct on x86 but wrong on Arm. >>> I expected there would be a check in do_arm_physdev_op() or somewhere >>> near it, similar to how it is done in x86, but there are none. I'm not >>> sure if it is intentional or by mistake, I think it needs some >>> clarification by Arm folks. >> >> Hmm, looking at x86'es do_physdev_op() I fear I can't see such a check there >> either. And indeed there are certain PHYSDEVOP_* which DomU-s may also make >> use of. > > It is one level above in hvm_physdev_op() > > case PHYSDEVOP_setup_gsi: > case PHYSDEVOP_pci_mmcfg_reserved: > case PHYSDEVOP_pci_device_add: > case PHYSDEVOP_pci_device_remove: > case PHYSDEVOP_pci_device_reset: > case PHYSDEVOP_dbgp_op: > if ( !is_hardware_domain(currd) ) > return -ENOSYS; > break;
But that's for just HVM guests, and only a functional restriction, not a security one. Jan
