lkml.org 
[lkml]   [2012]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Xen-devel] [PATCH] xen/apic: implement io apic read with hypercall
    On Fri, Apr 20, 2012 at 01:38:28PM +0100, Ian Campbell wrote:
    > On Fri, 2012-04-20 at 12:13 +0100, Lin Ming wrote:
    > > On Fri, 2012-04-20 at 10:58 +0100, Andrew Cooper wrote:
    > > > On 20/04/12 10:25, Lin Ming wrote:
    > > > > Implements xen_io_apic_read with hypercall, so it returns proper IO-APIC
    > > > > information instead of fabricated one.
    > > > >
    > > > > Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn>
    > > > > ---
    > > > > arch/x86/xen/apic.c | 16 +++++++++++-----
    > > > > 1 files changed, 11 insertions(+), 5 deletions(-)
    > > > >
    > > > > diff --git a/arch/x86/xen/apic.c b/arch/x86/xen/apic.c
    > > > > index aee16ab..f1f392d 100644
    > > > > --- a/arch/x86/xen/apic.c
    > > > > +++ b/arch/x86/xen/apic.c
    > > > > @@ -1,14 +1,20 @@
    > > > > #include <linux/init.h>
    > > > > #include <asm/x86_init.h>
    > > > > +#include <asm/apic.h>
    > > > > +#include <xen/interface/physdev.h>
    > > > > +#include <asm/xen/hypercall.h>
    > > > >
    > > > > unsigned int xen_io_apic_read(unsigned apic, unsigned reg)
    > > > > {
    > > > > - if (reg == 0x1)
    > > > > - return 0x00170020;
    > > > > - else if (reg == 0x0)
    > > > > - return apic << 24;
    > > > > + struct physdev_apic apic_op;
    > > > > + int ret;
    > > > >
    > > > > - return 0xff;
    > > > > + apic_op.apic_physbase = mpc_ioapic_addr(apic);
    > > > > + apic_op.reg = reg;
    > > > > + ret = HYPERVISOR_physdev_op(PHYSDEVOP_apic_read, &apic_op);
    > > > > + if (ret)
    > > > > + return ret;
    > > > > + return apic_op.value;
    > > >
    > > > Hypercall ret errors are negative, yet this function is unsigned. Given
    > > > that the previous function had no possible way to fail, perhaps on error
    > > > you should fake up the values as before.
    > >
    > > 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?

    >
    > Under what circumstances can these hypercalls fail? Would a BUG_ON be
    > appropriate/
    >
    > > unsigned int ret = apic_read(...);
    > > if (ret == -1)
    > > //handle error.
    > >
    > > Thanks,
    > > Lin Ming
    > >
    > >
    > >
    > > _______________________________________________
    > > Xen-devel mailing list
    > > Xen-devel@lists.xen.org
    > > http://lists.xen.org/xen-devel
    >
    >
    >
    > _______________________________________________
    > Xen-devel mailing list
    > Xen-devel@lists.xen.org
    > http://lists.xen.org/xen-devel


    \
     
     \ /
      Last update: 2012-04-20 18:49    [W:0.027 / U:31.216 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site