On 01/10/2024 2:35 pm, Jan Beulich wrote:
> On 01.10.2024 14:24, Andrew Cooper wrote:
>> Ever since it's introduction in commit 013351bd7ab3 ("Define new
>> event-channel
>> and physdev hypercalls"), the public interface was named nr_ports while the
>> internal field was called iobmp_limit.
>>
>> Rename the intenral field to iobmp_nr to match the public interface, and
>> clarify that, when nonzero, Xen will read 2 bytes.
>>
>> There isn't a perfect parallel with a real TSS, but iobmp_nr being 0 is the
>> paravirt "no IOPB" case, and it is important that no read occurs in this
>> case.
>>
>> Signed-off-by: Andrew Cooper <[email protected]>
> Reviewed-by: Jan Beulich <[email protected]>
Thanks.
> with one cosmetic request:
>
>> --- a/xen/arch/x86/pv/emul-priv-op.c
>> +++ b/xen/arch/x86/pv/emul-priv-op.c
>> @@ -167,7 +167,12 @@ static int guest_io_okay(unsigned int port, unsigned
>> int bytes,
>> if ( iopl_ok(v, regs) )
>> return X86EMUL_OKAY;
>>
>> - if ( (port + bytes) <= v->arch.pv.iobmp_limit )
>> + /*
>> + * When @iobmp_nr is non-zero, Xen, like real CPUs and the TSS IOPB,
>> + * always reads 2 bytes from @iobmp, which might be one byte beyond
>> + * @nr_ports.
>> + */
>> + if ( (port + bytes) <= v->arch.pv.iobmp_nr )
> If you use @nr_ports in the comment here, then I think some connection wants
> establishing as to where this comes from. Otherwise use @iobmp_nr a 2nd time.
Oops, that was a copy&paste error from the header. I'll switch it back
to @iobmp_nr.
~Andrew
