On Wed, Nov 06, 2024 at 08:05:19AM +0000, Mykyta Poturai wrote:
> During the construction of dmask value, it gets shifted by
> (32 - msi->vectors) bits. If msi->vectors is 0, the result of the shift
> becomes undefined due to shifting by a size of the type. While this
> works fine on x86, on ARM the resulting mask becomes 0xFFFFFFFF, which
> is incorrect.
>
> Fix this by adding an explicit check for msi->vectors == 0.
I would also add:
Fixes: 188fa82305e7 ('xen/vpci: Improve code generation in mask_write()')
> Signed-off-by: Mykyta Poturai <[email protected]>
> ---
> xen/drivers/vpci/msi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/drivers/vpci/msi.c b/xen/drivers/vpci/msi.c
> index 7bda47e7fc..787296fd42 100644
> --- a/xen/drivers/vpci/msi.c
> +++ b/xen/drivers/vpci/msi.c
> @@ -172,7 +172,7 @@ static void cf_check mask_write(
> struct vpci_msi *msi = data;
> uint32_t dmask = msi->mask ^ val;
>
> - if ( !dmask )
> + if ( !dmask || msi->vectors == 0 )
> return;
I'm afraid returning this early is not correct - the cached mask needs
to be updated, even if there are no vectors currently enabled.
The adjustment likely needs to be:
if ( msi->enabled && msi->vectors )
...
So that the update of msi->mask is not skipped.
Thanks, Roger.
