This patchset has two patches:
[1] 8-byte writes to non-mapped MMIO are broken into pairs of 4-byte writes, this patch makes such pairs atomic.

[2] Enable 8-byte accesses in vfio_region_write and vfio_region_read.

Patches based on master.

Jose Ricardo Ziviani (2):
  vfio: Set MemoryRegionOps:max_access_size and min_access_size
  vfio: enable 8-byte reads/writes to vfio

 hw/vfio/common.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

-- 
2.7.4

On 04/19/2017 12:44 PM, Jose Ricardo Ziviani wrote:
> This patchset has two patches:
> [1] 8-byte writes to non-mapped MMIO are broken into pairs of 4-byte writes, this patch makes such pairs atomic.
>
> [2] Enable 8-byte accesses in vfio_region_write and vfio_region_read.
>
> Patches based on master.
>
> Jose Ricardo Ziviani (2):
>   vfio: Set MemoryRegionOps:max_access_size and min_access_size
>   vfio: enable 8-byte reads/writes to vfio
>
>  hw/vfio/common.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>

I think these patches need to be squashed to be bisectable.


r~

On Thu, 20 Apr 2017 00:19:23 -0700
Richard Henderson <rth@twiddle.net> wrote:

> On 04/19/2017 12:44 PM, Jose Ricardo Ziviani wrote:
> > This patchset has two patches:
> > [1] 8-byte writes to non-mapped MMIO are broken into pairs of 4-byte writes, this patch makes such pairs atomic.
> >
> > [2] Enable 8-byte accesses in vfio_region_write and vfio_region_read.
> >
> > Patches based on master.
> >
> > Jose Ricardo Ziviani (2):
> >   vfio: Set MemoryRegionOps:max_access_size and min_access_size
> >   vfio: enable 8-byte reads/writes to vfio
> >
> >  hw/vfio/common.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >  
> 
> I think these patches need to be squashed to be bisectable.

No, I think it's fine.  The point of patch 1/2 is to indicate that the
hardware supports 8-byte accesses, which will still be broken into 2
4-byte accesses because we don't yet set the implemented width beyond
the default.  The important part is that the mutex will now group the 4
byte access pair together rather than letting them get re-ordered.
Patch 2/2 then implements native 8-byte access.  I appreciate them
being separate for this subtle nuance, but maybe I'm not seeing the
same issue as you.  Thanks,

Alex

On 20/04/2017 18:03, Alex Williamson wrote:
> On Thu, 20 Apr 2017 00:19:23 -0700
> Richard Henderson <rth@twiddle.net> wrote:
> 
>> On 04/19/2017 12:44 PM, Jose Ricardo Ziviani wrote:
>>> This patchset has two patches:
>>> [1] 8-byte writes to non-mapped MMIO are broken into pairs of 4-byte writes, this patch makes such pairs atomic.
>>>
>>> [2] Enable 8-byte accesses in vfio_region_write and vfio_region_read.
>>>
>>> Patches based on master.
>>>
>>> Jose Ricardo Ziviani (2):
>>>   vfio: Set MemoryRegionOps:max_access_size and min_access_size
>>>   vfio: enable 8-byte reads/writes to vfio
>>>
>>>  hw/vfio/common.c | 14 ++++++++++++++
>>>  1 file changed, 14 insertions(+)
>>>  
>>
>> I think these patches need to be squashed to be bisectable.
> 
> No, I think it's fine.  The point of patch 1/2 is to indicate that the
> hardware supports 8-byte accesses, which will still be broken into 2
> 4-byte accesses because we don't yet set the implemented width beyond
> the default.  The important part is that the mutex will now group the 4
> byte access pair together rather than letting them get re-ordered.
> Patch 2/2 then implements native 8-byte access.  I appreciate them
> being separate for this subtle nuance, but maybe I'm not seeing the
> same issue as you.  Thanks,
'
I agree, the patches looks fine as is.

Paolo

On 04/21/2017 03:06 AM, Paolo Bonzini wrote:
>
>
> On 20/04/2017 18:03, Alex Williamson wrote:
>> On Thu, 20 Apr 2017 00:19:23 -0700
>> Richard Henderson <rth@twiddle.net> wrote:
>>
>>> On 04/19/2017 12:44 PM, Jose Ricardo Ziviani wrote:
>>>> This patchset has two patches:
>>>> [1] 8-byte writes to non-mapped MMIO are broken into pairs of 4-byte writes, this patch makes such pairs atomic.
>>>>
>>>> [2] Enable 8-byte accesses in vfio_region_write and vfio_region_read.
>>>>
>>>> Patches based on master.
>>>>
>>>> Jose Ricardo Ziviani (2):
>>>>   vfio: Set MemoryRegionOps:max_access_size and min_access_size
>>>>   vfio: enable 8-byte reads/writes to vfio
>>>>
>>>>  hw/vfio/common.c | 14 ++++++++++++++
>>>>  1 file changed, 14 insertions(+)
>>>>
>>>
>>> I think these patches need to be squashed to be bisectable.
>>
>> No, I think it's fine.  The point of patch 1/2 is to indicate that the
>> hardware supports 8-byte accesses, which will still be broken into 2
>> 4-byte accesses because we don't yet set the implemented width beyond
>> the default.  The important part is that the mutex will now group the 4
>> byte access pair together rather than letting them get re-ordered.
>> Patch 2/2 then implements native 8-byte access.  I appreciate them
>> being separate for this subtle nuance, but maybe I'm not seeing the
>> same issue as you.  Thanks,
> '
> I agree, the patches looks fine as is.

Sorry, I misread the first patch, thinking that indicating hw support for 8 
byte required implementation of 8 byte.


r~

On Fri, Apr 21, 2017 at 12:06:15PM +0200, Paolo Bonzini wrote:
> 
> 
> On 20/04/2017 18:03, Alex Williamson wrote:
> > On Thu, 20 Apr 2017 00:19:23 -0700
> > Richard Henderson <rth@twiddle.net> wrote:
> > 
> >> On 04/19/2017 12:44 PM, Jose Ricardo Ziviani wrote:
> >>> This patchset has two patches:
> >>> [1] 8-byte writes to non-mapped MMIO are broken into pairs of 4-byte writes, this patch makes such pairs atomic.
> >>>
> >>> [2] Enable 8-byte accesses in vfio_region_write and vfio_region_read.
> >>>
> >>> Patches based on master.
> >>>
> >>> Jose Ricardo Ziviani (2):
> >>>   vfio: Set MemoryRegionOps:max_access_size and min_access_size
> >>>   vfio: enable 8-byte reads/writes to vfio
> >>>
> >>>  hw/vfio/common.c | 14 ++++++++++++++
> >>>  1 file changed, 14 insertions(+)
> >>>  
> >>
> >> I think these patches need to be squashed to be bisectable.
> > 
> > No, I think it's fine.  The point of patch 1/2 is to indicate that the
> > hardware supports 8-byte accesses, which will still be broken into 2
> > 4-byte accesses because we don't yet set the implemented width beyond
> > the default.  The important part is that the mutex will now group the 4
> > byte access pair together rather than letting them get re-ordered.
> > Patch 2/2 then implements native 8-byte access.  I appreciate them
> > being separate for this subtle nuance, but maybe I'm not seeing the
> > same issue as you.  Thanks,
> '
> I agree, the patches looks fine as is.
> 
> Paolo
> 

Hello!

Thank you all for your review but I have a quick question: is it ok for
merge? :)

Thanks

On Tue, 2 May 2017 23:41:01 -0300
joserz@linux.vnet.ibm.com wrote:

> On Fri, Apr 21, 2017 at 12:06:15PM +0200, Paolo Bonzini wrote:
> > 
> > 
> > On 20/04/2017 18:03, Alex Williamson wrote:  
> > > On Thu, 20 Apr 2017 00:19:23 -0700
> > > Richard Henderson <rth@twiddle.net> wrote:
> > >   
> > >> On 04/19/2017 12:44 PM, Jose Ricardo Ziviani wrote:  
> > >>> This patchset has two patches:
> > >>> [1] 8-byte writes to non-mapped MMIO are broken into pairs of 4-byte writes, this patch makes such pairs atomic.
> > >>>
> > >>> [2] Enable 8-byte accesses in vfio_region_write and vfio_region_read.
> > >>>
> > >>> Patches based on master.
> > >>>
> > >>> Jose Ricardo Ziviani (2):
> > >>>   vfio: Set MemoryRegionOps:max_access_size and min_access_size
> > >>>   vfio: enable 8-byte reads/writes to vfio
> > >>>
> > >>>  hw/vfio/common.c | 14 ++++++++++++++
> > >>>  1 file changed, 14 insertions(+)
> > >>>    
> > >>
> > >> I think these patches need to be squashed to be bisectable.  
> > > 
> > > No, I think it's fine.  The point of patch 1/2 is to indicate that the
> > > hardware supports 8-byte accesses, which will still be broken into 2
> > > 4-byte accesses because we don't yet set the implemented width beyond
> > > the default.  The important part is that the mutex will now group the 4
> > > byte access pair together rather than letting them get re-ordered.
> > > Patch 2/2 then implements native 8-byte access.  I appreciate them
> > > being separate for this subtle nuance, but maybe I'm not seeing the
> > > same issue as you.  Thanks,  
> > '
> > I agree, the patches looks fine as is.
> > 
> > Paolo
> >   
> 
> Hello!
> 
> Thank you all for your review but I have a quick question: is it ok for
> merge? :)

Yes, I've got it in a local branch, I'll send a pull request this
week.  Thanks,

Alex