Re: [PATCH] x86: guard against port I/O overlapping the RTC/CMOS range

[Jan Beulich <jbeulich@suse.com>]

On 20.07.2020 12:52, Roger Pau Monné wrote:
> On Fri, Jul 17, 2020 at 03:10:43PM +0200, Jan Beulich wrote:
>> Since we intercept RTC/CMOS port accesses, let's do so consistently in
>> all cases, i.e. also for e.g. a dword access to [006E,0071]. To avoid
>> the risk of unintended impact on Dom0 code actually doing so (despite
>> the belief that none ought to exist), also extend
>> guest_io_{read,write}() to decompose accesses where some ports are
>> allowed to be directly accessed and some aren't.
> 
> Wouldn't the same apply to displaced accesses to port 0xcf8?

No, CF8 is special - partial accesses have no meaning as to the
index selection for subsequent CFC accesses. Or else CF9
couldn't be a standalone port with entirely different
functionality..

>> @@ -373,25 +384,31 @@ static int read_io(unsigned int port, un
>>      return X86EMUL_OKAY;
>>  }
>>  
>> +static void _guest_io_write(unsigned int port, unsigned int bytes,
>> +                            uint32_t data)
> 
> There's nothing guest specific about this function I think? If so you
> could drop the _guest_ prefix and just name it io_write?

Hmm, when choosing the name I decided that (a) it's a helper of
the other function and (b) it's still guest driven data that we
output.

>> +{
>> +    switch ( bytes )
>> +    {
>> +    case 1:
>> +        outb((uint8_t)data, port);
>> +        if ( amd_acpi_c1e_quirk )
>> +            amd_check_disable_c1e(port, (uint8_t)data);
>> +        break;
>> +    case 2:
>> +        outw((uint16_t)data, port);
>> +        break;
>> +    case 4:
>> +        outl(data, port);
>> +        break;
>> +    }
> 
> Newlines after break statements would be nice, and maybe add a
> default: ASSERT_UNREACHABLE() case to be on the safe side?

Well, yes, I guess I should. But then if I edit this moved code,
I guess I'll also get rid of the stray casts.

Jan