Re: [Qemu-devel] [PATCH 5/5] megasas: LSI Megaraid SAS emulation

[Alexander Graf]

On 07/05/2011 03:59 PM, Paolo Bonzini wrote:
> On 07/05/2011 03:38 PM, Alexander Graf wrote:
> > > +        if (is_sgl64) {
> > > +            iov_pa = ldq_phys(pa);
> > > +        } else {
> > > +            iov_pa = ldl_phys(pa);
> > These load data from memory in target endianness. Are you sure that's
> > what you want? I'd expect this to be defined as little endian
> > (especially given that ldq and ldl on the same address work).
> Seems to be target endianness from the corresponding Linux code:
>
>          if (sge_count) {
>                  scsi_for_each_sg(scp, os_sgl, sge_count, i) {
>                          mfi_sgl->sge32[i].length = sg_dma_len(os_sgl);
>                          mfi_sgl->sge32[i].phys_addr = sg_dma_address(os_sgl);
>                  }
>          }
>
> ...
>
>          if (sge_count) {
>                  scsi_for_each_sg(scp, os_sgl, sge_count, i) {
>                          mfi_sgl->sge64[i].length = sg_dma_len(os_sgl);
>                          mfi_sgl->sge64[i].phys_addr = sg_dma_address(os_sgl);
>                  }
>          }
>
> Note that this is _either_ a ldq or a ldl depending on what the driver told
> the device.  It is not accessing a 64-bit value as 32-bit.

So how would the device know which endianness the target is then? This 
looks like broken Linux code to me then. Christoph, is the above correct 
for big endian systems?

Btw, reading through the Qemu sources, there is ldl_le_p() to read 
little endian values from memory. I haven't quite found one for _phys 
though. We can just add that however.


Alex