On 29.05.2023 10:39, Roger Pau Monné wrote:
> On Fri, May 26, 2023 at 09:35:04AM +0200, Jan Beulich wrote:
>> There's no point consuming two port-I/O slots. Even less so considering
>> that some real hardware permits both ports to be accessed in one go,
>> emulating of which requires there to be only a single instance.
>>
>> Signed-off-by: Jan Beulich <[email protected]>
>>
>> --- a/xen/arch/x86/hvm/vpic.c
>> +++ b/xen/arch/x86/hvm/vpic.c
>> @@ -377,25 +377,34 @@ static int cf_check vpic_intercept_elcr_
>> int dir, unsigned int port, unsigned int bytes, uint32_t *val)
>> {
>> struct hvm_hw_vpic *vpic;
>> - uint32_t data;
>> + unsigned int data, shift = 0;
>>
>> - BUG_ON(bytes != 1);
>> + BUG_ON(bytes > 2 - (port & 1));
>>
>> vpic = ¤t->domain->arch.hvm.vpic[port & 1];
>>
>> - if ( dir == IOREQ_WRITE )
>> - {
>> - /* Some IRs are always edge trig. Slave IR is always level trig. */
>> - data = *val & vpic_elcr_mask(vpic);
>> - if ( vpic->is_master )
>> - data |= 1 << 2;
>> - vpic->elcr = data;
>> - }
>> - else
>> - {
>> - /* Reader should not see hardcoded level-triggered slave IR. */
>> - *val = vpic->elcr & vpic_elcr_mask(vpic);
>> - }
>> + do {
>> + if ( dir == IOREQ_WRITE )
>> + {
>> + /* Some IRs are always edge trig. Slave IR is always level
>> trig. */
>> + data = (*val >> shift) & vpic_elcr_mask(vpic);
>> + if ( vpic->is_master )
>> + data |= 1 << 2;
>
> Not that you added this, but I'm confused. The spec I'm reading
> explicitly states that bits 0:2 are reserved and must be 0.
>
> Is this some quirk of the specific chipset we aim to emulate?
I don't think so. Note that upon reads the bit is masked out again.
Adding back further context, there's even a comment to this effect:
+ else
+ {
+ /* Reader should not see hardcoded level-triggered slave IR. */
+ data = vpic->elcr & vpic_elcr_mask(vpic);
The setting of the bit is solely for internal handling purposes,
aiui.
Jan
