lkml.org 
[lkml]   [2010]   [Jul]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] KVM: IOAPIC: only access APIC registers one dword at a time
    Hi, Xiao Guangrong

    Xiao Guangrong wrote:
    > The IOAPIC spec says:
    >
    > When accessing these registers, accesses must be done one dword at a time.
    > For example, software should never access byte 2 from the Data register before
    > accessing bytes 0 and 1. The hardware will not attempt to recover from a bad
    > programming model in this case.

    I could understand what you described on the above, but I don't think it is
    the best method to fix it. Could you consider a nice one?

    >
    > So, this patch removes other width access
    >
    > Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
    > ---
    > virt/kvm/ioapic.c | 10 +++-------
    > 1 files changed, 3 insertions(+), 7 deletions(-)
    >
    > diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
    > index 1149c60..f96816f 100644
    > --- a/virt/kvm/ioapic.c
    > +++ b/virt/kvm/ioapic.c
    > @@ -306,14 +306,10 @@ static int ioapic_mmio_read(struct kvm_io_device *this, gpa_t addr, int len,
    > spin_unlock(&ioapic->lock);
    >
    > switch (len) {
    > - case 8:
    > - *(u64 *) val = result;
    > - break;
    > - case 1:
    > - case 2:
    > case 4:
    > - memcpy(val, (char *)&result, len);

    Here the parameter len is not used for reading data from ioapic register, before switch case
    the value of ioapic register has been read by ioapic_read_indirect().


    > + *(u32 *) val = result;
    > break;
    > +
    > default:
    > printk(KERN_WARNING "ioapic: wrong length %d\n", len);

    And I think here is not good to print warning message. Maybe it is better to do
    such kind of checking at the first of this function before ioapic_read_indirect().

    > }
    > @@ -332,7 +328,7 @@ static int ioapic_mmio_write(struct kvm_io_device *this, gpa_t addr, int len,
    > (void*)addr, len, val);
    > ASSERT(!(addr & 0xf)); /* check alignment */
    >
    > - if (len == 4 || len == 8)
    > + if (len == 4)
    > data = *(u32 *) val;
    > else {
    > printk(KERN_WARNING "ioapic: Unsupported size %d\n", len);

    So I hope you could read the source code again. I think you can use a better method to fix it.

    Best Regards,
    Jin Dongming



    \
     
     \ /
      Last update: 2010-07-02 09:33    [W:0.024 / U:30.648 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site