Re: [PATCH] lpfc: correct writeq failures on 32-bit architectures

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 26, 2018 at 10:41 PM, James Smart <jsmart2021@xxxxxxxxx> wrote:
> On 2/26/2018 12:04 PM, Arnd Bergmann wrote:
>>
>> For the endianess, the key to understanding this is that readl/writel and
>> readq/writeq follow the convention of accessing data as little-endian
>> because
>> that is what 99% of MMIO accesses on PCI are: you have a 32-bit or 64-bit
>> register value that gets copied between a register and the CPU, and it's
>> up to the device manufacturer to implement that convention in hardware.
>> The Linux implementation of writeq() on all architectures therefore puts
>> the
>> LSB into the first byte of the output.
>
>
> ok, so I interpret this as readx/writex() get a cpu-endian-based value, and
> the routine compensates for cpu endianness as it puts it out on the io bus
> (LSB=1st byte). All well and good.

it helps to think of it as compensating for device endianess rather than
compensating for CPU endianess. The effect is the same of course.

>> In contrast, accessing memory using a pointer dereference is defined by
>> the CPU manufacturer, so in a big-endian CPU the first byte corresponds
>> to the MSB of the CPU register.
>>
>> In your loop
>>
>>                  for (i = 0; i < q->entry_size; i += sizeof(uint64_t))
>>                          writeq(*((uint64_t *)(tmp + i)), q->dpp_regaddr +
>> i);
>>
>> that ends up copying the first byte of 'tmp[]' to byte 7 of dpp_regaddr,
>> which makes no sense as the data in tmp is (presumably) just a stream of
>> bytes that we want to end up on a disk in the same order, regardless
>> of which mode the CPU happens to be in at the time we copy the data.
>
>
> This is where I get a bit lost.
>
> So what you're telling me is:
>   with
>     uint64_t a=5, *b;
>     *b=5;
>   that
>     writeq(a, regptr);
>   is not the same as
>     writeq(*b, regptr);
>   ??
>
> That doesn't make sense to me as "*ptr" should have resulted in a cpu-endian
> value being given to writeq(), ignorant of how that value came into
> existence (pointer/constant/value in register). Then writeq() does it's
> thing to ensure LSB=1st byte
>
> It would only make sense if writeq() can be a macro that such that it can
> resolve into something that is essentially
>   *regport = *b;
> thus it's treated as a bytestream starting at address b and does not
> necessarily compensate for the cpu-endianness of the multi-byte quantity.

What you are describing above is not a byte stream but dealing with
a 64-bit integer. In both cases you obviously end up with the destination
data being 0x05 0x00 0x00 0x00 0x00 0x00 0x00 0x00, there is no
difference.

The case that you have in the driver is more like

      char *bytestream = "this is a byte stream";
      u64 *tmp = (u64 *)bytestream;
      writeq(*tmp, regptr);
      writeq(*(tmp + 1), regptr + 1);

On a big-endian cpu, *tmp is the 64-bit number 0x7468697320697320,
and *(tmp +1) is 0x6120627974652073, as the access to the memory
location is performed using big-endian semantics.

When we write that to the PCI bus using little-endian semantics, the
buffer ends up being  0x20 0x73 0x69 0x20 0x73 0x69 0x68 0x74
0x20 0x73 0x20 0x65 0x74 0x79 0x62 0x20, or " si siht s etyb a".

        Arnd



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux