Re: [PATCH v5 3/7] x86/hvm: Allow access to registers on the same page as MSI-X table

From: Jan Beulich <jbeulich@suse.com>
To: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v5 3/7] x86/hvm: Allow access to registers on the same page as MSI-X table
Date: Mon, 29 Apr 2024 08:37:56 +0200	[thread overview]
Message-ID: <59a59477-f343-454a-9f83-e9df28286065@suse.com> (raw)
In-Reply-To: <ZivHw9RUUN1CV4Hi@mail-itl>

On 26.04.2024 17:26, Marek Marczykowski-Górecki wrote:
> On Thu, Apr 25, 2024 at 01:15:34PM +0200, Jan Beulich wrote:
>> On 13.03.2024 16:16, Marek Marczykowski-Górecki wrote:
>>> +    hwaddr = fix_to_virt(fixmap_idx) + PAGE_OFFSET(address);
>>> +
>>> +    switch ( len )
>>> +    {
>>> +    case 1:
>>> +        *pval = readb(hwaddr);
>>> +        break;
>>> +
>>> +    case 2:
>>> +        *pval = readw(hwaddr);
>>> +        break;
>>> +
>>> +    case 4:
>>> +        *pval = readl(hwaddr);
>>> +        break;
>>> +
>>> +    case 8:
>>> +        *pval = readq(hwaddr);
>>> +        break;
>>> +
>>> +    default:
>>> +        ASSERT_UNREACHABLE();
>>
>> Misra demands "break;" to be here for release builds. In fact I wonder
>> why "*pval = ~0UL;" isn't put here, too. Question of course is whether
>> in such a case a true error indicator wouldn't be yet better.
> 
> I don't think it possible for the msixtbl_read() (that calls
> adjacent_read()) to be called with other sizes.

I agree, but scanners won't know.

Jan

> The default label is here exactly to make it obvious for the reader.
> 