On Friday 12 November 2010 00:12:21 Michael S. Tsirkin wrote:
> On Thu, Nov 11, 2010 at 03:47:00PM +0800, Sheng Yang wrote:
> > Add marco for big-endian machine.(Untested!)
> >
> > Signed-off-by: Sheng Yang <[email protected]>
>
> I presume this is tested at the same level as the previous patch?
> So you want to fold this into the previous patch.
Of course tested on Intel's platform, but it didn't make sense for big-endian
patch...
I'd like to make it separate still, because I can't test it properly.
>
> Also, please build with sparse (C=1) to find some endian-ness issues.
Sure. (Have it run with the patches, nothing new found...)
>
> > ---
> >
> > virt/kvm/assigned-dev.c | 7 ++++---
> > 1 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/virt/kvm/assigned-dev.c b/virt/kvm/assigned-dev.c
> > index 3010d7d..15b5f74 100644
> > --- a/virt/kvm/assigned-dev.c
> > +++ b/virt/kvm/assigned-dev.c
> > @@ -848,9 +848,9 @@ static int msix_mmio_read(struct kvm_io_device *this,
> > gpa_t addr, int len,
> >
> > idx = (addr - adev->msix_mmio_base) / PCI_MSIX_ENTRY_SIZE;
> > if ((addr % PCI_MSIX_ENTRY_SIZE) ==
> >
> > PCI_MSIX_ENTRY_VECTOR_CTRL)
> >
> > - *(unsigned long *)val =
> > + *(unsigned long *)val = le32_to_cpu(
>
> This should be cpu_to_le32. And val cast to __le32.
I think we can just cast val to __le32 here? The result should be big-endian.
No, no, I suppose all parameter of msix_mmio_read/write should be little-endian
because it's being written into PCI MSI-X table?
>
> > test_bit(idx, adev->msix_mask_bitmap) ?
> >
> > - PCI_MSIX_ENTRY_CTRL_MASKBIT : 0;
> > + PCI_MSIX_ENTRY_CTRL_MASKBIT : 0);
> >
> > else
> >
> > r = -EOPNOTSUPP;
> >
> > goto out;
>
> In this function,
> entry must be __le32 too, and fix up endian-ness where you fill it in.
>
> > @@ -869,6 +869,7 @@ static int msix_mmio_read(struct kvm_io_device *this,
> > gpa_t addr, int len,
> >
> > adev->msix_mask_bitmap);
> >
> > memcpy(val, &entry[addr % PCI_MSIX_ENTRY_SIZE / sizeof *entry], len);
> >
> > + *(unsigned long *)val = le32_to_cpu(*(unsigned long *)val);
> >
> > out:
> > mutex_unlock(&adev->kvm->lock);
> > return r;
> >
> > @@ -881,7 +882,7 @@ static int msix_mmio_write(struct kvm_io_device
> > *this, gpa_t addr, int len,
> >
> > container_of(this, struct kvm_assigned_dev_kernel,
> >
> > msix_mmio_dev);
> >
> > int idx, r = 0;
> >
> > - unsigned long new_val = *(unsigned long *)val;
> > + unsigned long new_val = cpu_to_le32(*(unsigned long *)val);
>
> __le32
Should it be
unsigned long new_val = le32_to_cpu(*(unsigned long *)val);
*val should be little endian I guess?
Or Michael, could you provide a patch for this? I really don't think I can
guarantee correctness of this patch...
--
regards
Yang, Sheng
>
> > mutex_lock(&adev->kvm->lock);
> > if (!msix_mmio_in_range(adev, addr, len)) {
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
