Re: [Xen-devel] [PATCH] xen/apic: implement io apic read with hypercall

[Lin Ming <mlin@ss.pku.edu.cn>]

On Mon, Apr 23, 2012 at 11:11 PM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
>> >> > How about return -1 on error?
>> >> > The calling function can check -1 for error.
>> >>
>> >> Isn't -1 potentially (at least theoretically) a valid value to read from
>> >> one of these registers?
>> >
>> > Yeah, but then we are back to crashing at bootup (with dom0) :-)
>> >
>> > Perhaps the fallback is to emulate (so retain some of the original code)
>> > as we have been since .. uh 3.0?
>>
>> Do you mean the return value of io_apic_read in 3.0?
>
> No. I meant that we would continue to emulate. The improvement
> is that now we do:
>
>       if (reg == 0x1)
>               return 0x00170020;
>       else if (reg == 0x0)
>               return apic << 24;
>
> instead of 0xfdfdfdfd.
>
>> It's 0xffffffff.
>
> Now it is 0xfdfdfdfd.
>
> I am suggesting that instead of BUG_ON(), we fallback to do returning
> an emulatated IO_APIC values - like the ones that this original patch
> cooked up..

But we still need to return some value if the register is not emulated.

How about below?

unsigned int xen_io_apic_read(unsigned apic, unsigned reg)
{
        struct physdev_apic apic_op;
        int ret;

        apic_op.apic_physbase = mpc_ioapic_addr(apic);
        apic_op.reg = reg;
        ret = HYPERVISOR_physdev_op(PHYSDEVOP_apic_read, &apic_op);
        if (!ret)
                return apic_op.value;

        /* emulate register */
        if (reg == 0x1)
                return 0x00170020;
        else if (reg == 0x0)
                return apic << 24;
        else
                return -1;
}