Re: [Xen-devel] [PATCH 2/4] x86/HVM: replace plain numbers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.01.15 at 15:41, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 22/01/15 13:57, Jan Beulich wrote:
>> ... making the code better document itself. No functional change
>> intended.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> --- a/xen/arch/x86/hvm/vioapic.c
>> +++ b/xen/arch/x86/hvm/vioapic.c
>> @@ -53,18 +53,26 @@ static uint32_t vioapic_read_indirect(co
>>      switch ( vioapic->ioregsel )
>>      {
>>      case VIOAPIC_REG_VERSION:
>> -        result = ((((VIOAPIC_NUM_PINS-1) & 0xff) << 16)
>> -                  | (VIOAPIC_VERSION_ID & 0xff));
>> +        result = ((union IO_APIC_reg_01){
>> +                  .bits = { .version = VIOAPIC_VERSION_ID,
>> +                            .entries = VIOAPIC_NUM_PINS - 1 }
>> +                  }).raw;
>>          break;
>>  
>>      case VIOAPIC_REG_APIC_ID:
>> +        /*
>> +         * Using union IO_APIC_reg_02 for the ID register too, as
>> +         * union IO_APIC_reg_00's ID field is 8 bits wide for some reason.
>> +         */
> 
> Having looked into this, Intel has a 4 bit wide ID with the top 4 bits
> reserved, while AMD has the top 4 bits as the Extended ID which may be
> used if an appropriate northbridge register has been set.
> 
> I think it might be better to use IO_APIC_reg_00 here and mask the write
> operations, leaving a note about Intel vs AMD and the fact that emulate
> an Intel IOAPIC to match the PIIX3 chipset in Qemu.
> 
> Modifying IO_APIC_reg_00 is not appropriate as Xens IOAPIC code needs to
> deal with AMD systems as well.

I had it that way first, but for the purpose of making very clear that
there is no functional change, I decided against doing such a conversion.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel