[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] x86/HVM: adjust pIRQ calculation in hvm_inject_msi()
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Date: Mon, 17 Jul 2023 14:47:49 +0200
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ozRtPdw8dBfKo/xT+r1wXFtk9WR06Jl6ff4YMjmpPnk=; b=EX5XAsYXbkQghTh0ftWfWqMKA230NAF5CMm0syfBRlFq+KAR0k2PNe67Wy9iHx61U2lp9PxHL2NEvVWnR2aodhyzqcV1PM1lDYIAsODEnMrxhxUnztE9aAsGJ0gL2b8/NUdDAsQDrD+w5+yfVjJxURH4HxC3VI6VSgDYwUSLx1wMYxfDb4dwLUUIayWmJURZo84tq/LBM2/MiTn3vpxnNckOE3Ow+8lEvfQeR1gG5Qedfj1wu2pZlAvfhWhHAD0lM7B3Fol3XeKYGwSguNHZ3OYDPwzDwqa0T4ZKa7EzWem5x8XzwYWtq7uZ7MRUunoJhdsVM49g1/cGj7GFVPo/Sw==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=f50m+bAsX4B+zQeKMSdBczgynViXKZDXKzFBK+1xGxltFc7JdUBBCnO4i0UaMM5JQ/AK0RJSduSKzV45hV2niDmaHU9BLflxVofrG2USxp9vQHhzb7YaRFuFo07W4b83BLmEmv0Fan8rnLqAwCPEEOJES6IdrVjIC695bnwwO8MQRX0+6phVom3GbhSAQYh5FuCLEXBdtBqG6flS/ras2RsS22zO/yVr2vml3C4OtNGQWXAyfNhACEsWRJHq2CqNPVIuSjeiDzmAXksO1I9bpj64P1RTCEOZPbZHGFhcl5pbEyuGwKypGcJXpKJhhi7I7TsDhtZIPPXLMC6bJrDPcg==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Simone Ballarin <simone.ballarin@xxxxxxxxxxx>, Gianluca Luparini <gianluca.luparini@xxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>
- Delivery-date: Mon, 17 Jul 2023 12:48:19 +0000
- Ironport-data: A9a23:WQOb4qAjB+T7UBVW/9viw5YqxClBgxIJ4kV8jS/XYbTApGkqhjVWy mdJDGmCPaqPZzbwc9l1Oti29ktT68KGyoc2QQY4rX1jcSlH+JHPbTi7wuUcHAvJd5GeExg3h yk6QoOdRCzhZiaE/n9BCpC48T8nk/nOHuGmYAL9EngZbRd+Tys8gg5Ulec8g4p56fC0GArIs t7pyyHlEAbNNwVcbCRMsspvlDs15K6p4GxB4QRnDRx2lAS2e0c9Xcp3yZ6ZdxMUcqEMdsamS uDKyq2O/2+x13/B3fv8z94X2mVTKlLjFVDmZkh+AsBOsTAbzsAG6Y4pNeJ0VKtio27hc+ada jl6ncfYpQ8BZsUgkQmGOvVSO3kW0aZuoNcrLZUj2CA6IoKvn3bEmp1T4E8K0YIwuedKDUQe7 fgjETEGTxGyq9q65K+Xc7w57igjBJGD0II3nFhFlGmcKMl8BJfJTuPN+MNS2yo2ioZWB/HCa sEFaD1pKhPdfxlIPVRRA5U79AuqriCnL3sE9xTI+Oxuuze7IA9ZidABNPLPfdOHX4NNl1uwr WPa5WXpRBodMbRzzBLcqyzw2bSfwHmTtIQ6EuO46P16jFmv32lMIhsvf3yKqt7isxvrMz5YA wlOksY0loAw/kG2Stj2XzWjvWWJ+BUbXrJ4A+A8rQ2A1KfQywKYHXQfCC5MbsQ8s807TiBs0 UWG9/v2ARR/vbvTTmiSnp+PrDa1PyVTJ2YGYSYeTA0t6cTsu4w1gVTESdMLOKG0h9vxBDr5h TSXtCEkhrMSpcQW2L62+1+BiDWpzrDSVRI87AjTWmOj7yt6aZSjaoju7kLUhcusN66cR1iF+ X0bwc6X6blWCYnXzXTSBuIQALuu+vCJdiXGhkJiFIUg8DLr/GO/eYdX43d1I0IB3ts4RAIFq XT74Wt5jKK/9lPzBUOrS+pd0/gX8JU=
- Ironport-hdrordr: A9a23:cSr54aokiHNiBdfQ5hk8oGoaV5oweYIsimQD101hICG9Ffbo9f xG/c5rtiMc5wx6ZJhNo7290ey7L080lqQU3WByB9iftWDd0QPCEGgI1+rfKlPbdxHDyg==
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On Mon, Jul 17, 2023 at 01:49:43PM +0200, Jan Beulich wrote:
> On 17.07.2023 12:51, Roger Pau Monné wrote:
> > 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 <jbeulich@xxxxxxxx>
> >
> > Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>
> Thanks.
>
> > 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?
>
> I think 0 is okay to use here, as the low IRQs (at least the 16 ISA
> ones) are all 1:1 mapped to their "machine" (i.e. Xen's) IRQ numbers.
> And IRQ0 is always the timer, never given to guests (not even to
> Dom0).
I'm kind of confused by this not being dom0, but rather an
HVM guest, so pIRQ 0 of that HVM guest should be available to the
guest itself?
IOW: the possible values here should be the full pIRQ range, as there
are never Xen owned pIRQs in the context of an HVM guest. One further
limitation is that even in that case pIRQs for (guest) GSIs would
still be identity mapped, so GSI 0 won't be a suitable pIRQ for an MSI
source.
The usage of pIRQs here even for emulated devices makes me very
confused.
Thanks, Roger.
|
