On Mon, Jul 17, 2023 at 11:31:57AM +0200, Jan Beulich wrote:
> While the referenced commit came without any update to the public header
> (which doesn't clarify how the upper address bits are used), the
> intention looks to have been that bits 12..19 and 40..63 form the pIRQ.
> Negative values simply make no sense, and pirq_info() also generally
> wants invoking with an unsigned (and not just positive) value.
>
> Since the line was pointed out by Eclair, address Misra rule 7.2 at the
> same time, by adding the missing U suffix.
>
> Fixes: 88fccdd11ca0 ("xen: event channel remapping for emulated MSIs")
> Signed-off-by: Jan Beulich <[email protected]>
Acked-by: Roger Pau Monné <[email protected]>
I have a question below, but not related to the change here.
>
> --- a/xen/arch/x86/hvm/irq.c
> +++ b/xen/arch/x86/hvm/irq.c
> @@ -383,7 +383,7 @@ int hvm_inject_msi(struct domain *d, uin
>
> if ( !vector )
> {
> - int pirq = ((addr >> 32) & 0xffffff00) | dest;
> + unsigned int pirq = ((addr >> 32) & 0xffffff00U) | dest;
>
> if ( pirq > 0 )
I do wonder whether this check is also accurate, as pIRQ 0 could be a
valid value. Should it instead use INVALID_PIRQ?
Since there's no specification or documentation how this is supposed
to work it's hard to tell.
Thanks, Roger.
