[v3] x86/PVH: Dom0 building adjustments

[PATCH v3 3/9] x86/PVH: permit more physdevop-s to be used by Dom0

Posted by Jan Beulich 3 years, 2 months ago

Certain notifications of Dom0 to Xen are independent of the mode Dom0 is
running in. Permit further PCI related ones (only their modern forms).
Also include the USB2 debug port operation at this occasion.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I'm uncertain about the has_vpci() part of the check: I would think
is_hardware_domain() is both sufficient and concise. Without vPCI a PVH
Dom0 won't see any PCI devices in the first place (and hence would
effectively be non-functioning). Dropping this would in particular make
PHYSDEVOP_dbgp_op better fit in the mix.
---
v3: New.

--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -94,6 +94,12 @@ static long hvm_physdev_op(int cmd, XEN_
         break;
 
     case PHYSDEVOP_pci_mmcfg_reserved:
+    case PHYSDEVOP_pci_device_add:
+    case PHYSDEVOP_pci_device_remove:
+    case PHYSDEVOP_restore_msi_ext:
+    case PHYSDEVOP_dbgp_op:
+    case PHYSDEVOP_prepare_msix:
+    case PHYSDEVOP_release_msix:
         if ( !has_vpci(currd) || !is_hardware_domain(currd) )
             return -ENOSYS;
         break;

Re: [PATCH v3 3/9] x86/PVH: permit more physdevop-s to be used by Dom0

Posted by Roger Pau Monné 3 years, 2 months ago

On Tue, Sep 21, 2021 at 09:17:37AM +0200, Jan Beulich wrote:
> Certain notifications of Dom0 to Xen are independent of the mode Dom0 is
> running in. Permit further PCI related ones (only their modern forms).
> Also include the USB2 debug port operation at this occasion.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> I'm uncertain about the has_vpci() part of the check: I would think
> is_hardware_domain() is both sufficient and concise. Without vPCI a PVH
> Dom0 won't see any PCI devices in the first place (and hence would
> effectively be non-functioning). Dropping this would in particular make
> PHYSDEVOP_dbgp_op better fit in the mix.
> ---
> v3: New.
> 
> --- a/xen/arch/x86/hvm/hypercall.c
> +++ b/xen/arch/x86/hvm/hypercall.c
> @@ -94,6 +94,12 @@ static long hvm_physdev_op(int cmd, XEN_
>          break;
>  
>      case PHYSDEVOP_pci_mmcfg_reserved:
> +    case PHYSDEVOP_pci_device_add:
> +    case PHYSDEVOP_pci_device_remove:
> +    case PHYSDEVOP_restore_msi_ext:

Hm, I'm slightly unsure we need the restore operation. Wouldn't it be
better to just reset all device state on suspend and then let dom0
restore it's state as it does on native?

Maybe there's some wrinkle that prevents that from working properly.

> +    case PHYSDEVOP_dbgp_op:
> +    case PHYSDEVOP_prepare_msix:
> +    case PHYSDEVOP_release_msix:

Albeit I think those two operations won't strictly conflict with vPCI
usage (as they require no MSIX entries to be activ) I still wonder
whether we will end up needing them on a PVH dom0. They are used by
pciback and it's not yet clear how we will end up using pciback on a
PVH dom0, hence I would prefer if we could leave them out until
strictly required.

Thanks, Roger.

Re: [PATCH v3 3/9] x86/PVH: permit more physdevop-s to be used by Dom0

Posted by Jan Beulich 3 years, 2 months ago

On 22.09.2021 16:22, Roger Pau Monné wrote:
> On Tue, Sep 21, 2021 at 09:17:37AM +0200, Jan Beulich wrote:
>> Certain notifications of Dom0 to Xen are independent of the mode Dom0 is
>> running in. Permit further PCI related ones (only their modern forms).
>> Also include the USB2 debug port operation at this occasion.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> I'm uncertain about the has_vpci() part of the check: I would think
>> is_hardware_domain() is both sufficient and concise. Without vPCI a PVH
>> Dom0 won't see any PCI devices in the first place (and hence would
>> effectively be non-functioning). Dropping this would in particular make
>> PHYSDEVOP_dbgp_op better fit in the mix.
>> ---
>> v3: New.
>>
>> --- a/xen/arch/x86/hvm/hypercall.c
>> +++ b/xen/arch/x86/hvm/hypercall.c
>> @@ -94,6 +94,12 @@ static long hvm_physdev_op(int cmd, XEN_
>>          break;
>>  
>>      case PHYSDEVOP_pci_mmcfg_reserved:
>> +    case PHYSDEVOP_pci_device_add:
>> +    case PHYSDEVOP_pci_device_remove:
>> +    case PHYSDEVOP_restore_msi_ext:
> 
> Hm, I'm slightly unsure we need the restore operation. Wouldn't it be
> better to just reset all device state on suspend and then let dom0
> restore it's state as it does on native?

Hmm - Linux (even after my patch separating XEN_DOM0 from XEN_PV)
only issues this call when running in PV mode, so from that
perspective leaving it out would be okay. (Otherwise, i.e. if we
decide to permit its use, I guess we would better also permit
PHYSDEVOP_restore_msi. Somehow I had managed to not spot that.)
However, ...

> Maybe there's some wrinkle that prevents that from working properly.

... Xen might be using MSI for the serial console, and I'm not sure
this interrupt would get properly re-setup.

>> +    case PHYSDEVOP_dbgp_op:
>> +    case PHYSDEVOP_prepare_msix:
>> +    case PHYSDEVOP_release_msix:
> 
> Albeit I think those two operations won't strictly conflict with vPCI
> usage (as they require no MSIX entries to be activ) I still wonder
> whether we will end up needing them on a PVH dom0. They are used by
> pciback and it's not yet clear how we will end up using pciback on a
> PVH dom0, hence I would prefer if we could leave them out until
> strictly required.

Even without a clear plan towards pciback, do you have any idea how
their function could sensibly be replaced in the PVH case? If there
is at least a rough idea, I'd be fine leaving them out here.

Jan