[SLOF] [PATCH v3 14/22] virtio: add 64-bit virtio helpers for 1.0

[Thomas Huth]

On 27.01.2016 14:30, Nikunj A Dadhania wrote:
> Thomas Huth <thuth at redhat.com> writes:
> 
>> On 22.01.2016 11:54, Nikunj A Dadhania wrote:
>>> 64-bit fields are to be treated as two 32-bit fields, with low 32 bit
>>> part followed by the high 32 bit part.
>>>
>>> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
>>> ---
>>>  lib/libvirtio/virtio.c | 25 +++++++++++++++++++++++++
>>>  1 file changed, 25 insertions(+)
>>>
>>> diff --git a/lib/libvirtio/virtio.c b/lib/libvirtio/virtio.c
>>> index 8c22e92..40bbd37 100644
>>> --- a/lib/libvirtio/virtio.c
>>> +++ b/lib/libvirtio/virtio.c
>>> @@ -51,6 +51,31 @@ struct virtio_dev_common {
>>>  	le64 q_used;
>>>  } __attribute__ ((packed));
>>>  
>>> +/* virtio 1.0 Spec: 4.1.3 PCI Device Layout
>>> + *
>>> + * Fields of different sizes are present in the device configuration regions.
>>> + * All 64-bit, 32-bit and 16-bit fields are little-endian. 64-bit fields are to
>>> + * be treated as two 32-bit fields, with low 32 bit part followed by the high 32
>>> + * bit part.
>>> + */
>>> +static void virtio_pci_write64(void *addr, uint64_t val)
>>> +{
>>> +	uint32_t hi = (val >> 32) & 0xFFFFFFFF;
>>> +	uint32_t lo = val & 0xFFFFFFFF;
>>> +
>>> +	ci_write_32(addr, cpu_to_le32(lo));
>>> +	ci_write_32(addr + 4, cpu_to_le32(hi));
>>> +}
>>> +
>>> +static uint64_t virtio_pci_read64(void *addr)
>>> +{
>>> +	uint64_t hi, lo;
>>> +
>>> +	lo = le32_to_cpu(ci_read_32(addr));
>>> +	hi = le32_to_cpu(ci_read_32(addr + 4));
>>> +	return (hi << 32) | lo;
>>> +}
>>
>> So after the discussion with Alexey in the other mail thread, I'm a
>> little bit confused now ... are these functions here now only used for
>> PCI config space or also for MMIO?
> 
> It is used only for PCI config space.  
> 
>> If they are used for config space only, I think I'd rather agree with
>> Alexey and move them to slof/helpers.c, rename them to
>> SLOF_pci_config_read/write64 and use forth_eval("config-l!") etc. for
>> consistency ...
> 
> config-l! is for 32-bit registers.
> 
> There is no config-x{!,@} (8bytes) version, we would need to add it in that
> case.

I thought something like this:

long SLOF_pci_config_read64(long offset)
{
	uint64_t hi, lo;
        forth_push(offset + 4);
	forth_push(offset);
        forth_eval("config-l@ config-l@");
	lo = forth_pop();
	hi = forth_pop();
        return (hi << 32) | lo;
}

... but looking at the implementation of config-l@ (it uses
rtas-do-config-@, I think), it might be better to use your approach with
ci_read_32() here instead (to guarantee the atomicity of the 32-bit
accesses). So it's also likely better to really keep the functions here
instead of moving them to helpers.c.

Reviewed-by: Thomas Huth <thuth at redhat.com>