Re: [PATCH 13/24] x86: restrict PHYSDEVOP_* when PV=n

On 7/28/26 9:19 AM, Jan Beulich wrote:
> hvm_physdev_op() permits through only a subset of sub-ops. The code
> handling other sub-ops is therefore unreachable when PV=n, violating MISRA
> C:2012 rule 2.1. With that the XSM .apic() hook also becomes unreachable /
> dead when PV=n.
> 
> Signed-off-by: Jan Beulich <[email protected]>
> ---
> At least for the sub-ops using xsm_apic() IS_ENABLED() cannot be used.
> Therefore #ifdef is used throughout.
> 
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -233,6 +233,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
>           break;
>       }
>   
> +#ifdef CONFIG_PV
> +
>       case PHYSDEVOP_pirq_eoi_gmfn_v2:
>       case PHYSDEVOP_pirq_eoi_gmfn_v1: {
>           struct physdev_pirq_eoi_gmfn info;
> @@ -281,6 +283,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
>           break;
>       }
>   
> +#endif /* CONFIG_PV */
> +
>       case PHYSDEVOP_irq_status_query: {
>           struct physdev_irq_status_query irq_status_query;
>           ret = -EFAULT;
> @@ -379,6 +383,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
>           break;
>       }
>   
> +#ifdef CONFIG_PV
> +
>       case PHYSDEVOP_apic_read: {
>           struct physdev_apic apic;
>           ret = -EFAULT;
> @@ -524,6 +530,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
>           break;
>       }
>   
> +#endif /* CONFIG_PV */
> +
>       case PHYSDEVOP_pci_mmcfg_reserved: {
>           struct physdev_pci_mmcfg_reserved info;
>   
> @@ -558,6 +566,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
>           break;
>       }
>   
> +#ifdef CONFIG_PV
> +
>       case PHYSDEVOP_restore_msi: {
>           struct physdev_restore_msi restore_msi;
>           struct pci_dev *pdev;
> @@ -589,6 +599,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
>           break;
>       }
>   
> +#endif /* CONFIG_PV */
> +
>       case PHYSDEVOP_setup_gsi: {
>           struct physdev_setup_gsi setup_gsi;
>   
> @@ -608,6 +620,7 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H
>                                 setup_gsi.polarity);
>           break;
>       }
> +
>       case PHYSDEVOP_get_free_pirq: {
>           struct physdev_get_free_pirq out;
>   
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -648,13 +648,6 @@ static XSM_INLINE int cf_check xsm_mem_s
>       return xsm_default_action(action, current->domain, cd);
>   }
>   
> -static XSM_INLINE int cf_check xsm_apic(
> -    XSM_DEFAULT_ARG struct domain *d, int cmd)
> -{
> -    XSM_ASSERT_ACTION(XSM_PRIV);
> -    return xsm_default_action(action, d, NULL);
> -}
> -
>   static XSM_INLINE int cf_check xsm_machine_memory_map(XSM_DEFAULT_VOID)
>   {
>       XSM_ASSERT_ACTION(XSM_PRIV);
> @@ -670,6 +663,13 @@ static XSM_INLINE int cf_check xsm_domai
>   
>   #ifdef CONFIG_PV
>   
> +static XSM_INLINE int cf_check xsm_apic(
> +    XSM_DEFAULT_ARG struct domain *d, int cmd)
> +{
> +    XSM_ASSERT_ACTION(XSM_PRIV);
> +    return xsm_default_action(action, d, NULL);
> +}
> +
>   static XSM_INLINE int cf_check xsm_do_mca(XSM_DEFAULT_VOID)
>   {
>       XSM_ASSERT_ACTION(XSM_PRIV);
> --- a/xen/include/xsm/hooks.h
> +++ b/xen/include/xsm/hooks.h
> @@ -131,10 +131,10 @@ XSM_HOOK(int, mem_sharing_op, struct dom
>   XSM_HOOK(int, platform_op, uint32_t)
>   
>   #ifdef CONFIG_X86
> -XSM_HOOK(int, apic, struct domain *, int)
>   XSM_HOOK(int, machine_memory_map)
>   XSM_HOOK(int, domain_memory_map, struct domain *)
>   #ifdef CONFIG_PV
> +XSM_HOOK(int, apic, struct domain *, int)
>   XSM_HOOK(int, do_mca)
>   XSM_HOOK(int, mmu_update, struct domain *, struct domain *, struct domain *,
>                             uint32_t)
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1749,6 +1749,19 @@ static int cf_check flask_mem_sharing_op
>   }
>   #endif
>   
> +static int cf_check flask_machine_memory_map(void)
> +{
> +    return avc_current_has_perm(SECINITSID_XEN, SECCLASS_MMU, MMU__MEMORYMAP,
> +                                NULL);
> +}
> +
> +static int cf_check flask_domain_memory_map(struct domain *d)
> +{
> +    return current_has_perm(d, SECCLASS_MMU, MMU__MEMORYMAP);
> +}
> +
> +#ifdef CONFIG_PV
> +
>   static int cf_check flask_apic(struct domain *d, int cmd)
>   {
>       uint32_t perm;
> @@ -1769,18 +1782,6 @@ static int cf_check flask_apic(struct do
>       return domain_has_xen(d, perm);
>   }
>   
> -static int cf_check flask_machine_memory_map(void)
> -{
> -    return avc_current_has_perm(SECINITSID_XEN, SECCLASS_MMU, MMU__MEMORYMAP, NULL);
> -}
> -
> -static int cf_check flask_domain_memory_map(struct domain *d)
> -{
> -    return current_has_perm(d, SECCLASS_MMU, MMU__MEMORYMAP);
> -}
> -
> -#ifdef CONFIG_PV
> -
>   static int cf_check flask_do_mca(void)
>   {
>       return domain_has_xen(current->domain, XEN__MCA_OP);
> 

Acked-by: Daniel P. Smith <[email protected]>