Re: [Qemu-devel] [PATCH 0/3] Fix access_with_adjusted_size() on big-endian

Re: [Qemu-devel] [PATCH 0/3] Fix access_with_adjusted_size() on big-endian

[Peter Maydell]

On 27 September 2018 at 01:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Hi,
>
> This series fix a bug I'v been hunting for a long time.
>
> With BE regions, if the guest used smaller access than the region
> implementation, the shift value is negative, but since access_fn()
> uses unsigned type for shift, it result in a huge positive value,
> then accessors shift the value which eventually becomes 0.
>
> The fix is simply to use signed type for the shift, and shift to
> the opposite direction for negative values.

Series
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I guess we don't have very many devices that are BE and
that allow the guest to access them with a smaller
width than their implemented width...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] Fix access_with_adjusted_size() on big-endian

[Philippe Mathieu-Daudé]

On Tue, Oct 2, 2018 at 2:19 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> On 27 September 2018 at 01:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > Hi,
> >
> > This series fix a bug I'v been hunting for a long time.
> >
> > With BE regions, if the guest used smaller access than the region
> > implementation, the shift value is negative, but since access_fn()
> > uses unsigned type for shift, it result in a huge positive value,
> > then accessors shift the value which eventually becomes 0.
> >
> > The fix is simply to use signed type for the shift, and shift to
> > the opposite direction for negative values.
>
> Series
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Thanks for reviewing this.

> I guess we don't have very many devices that are BE and
> that allow the guest to access them with a smaller
> width than their implemented width...

Do you think is it worth using the '(un)likely()' macros?

Re: [Qemu-devel] [PATCH 0/3] Fix access_with_adjusted_size() on big-endian

[Peter Maydell]

On 2 October 2018 at 13:34, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On Tue, Oct 2, 2018 at 2:19 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 27 September 2018 at 01:24, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> > Hi,
>> >
>> > This series fix a bug I'v been hunting for a long time.
>> >
>> > With BE regions, if the guest used smaller access than the region
>> > implementation, the shift value is negative, but since access_fn()
>> > uses unsigned type for shift, it result in a huge positive value,
>> > then accessors shift the value which eventually becomes 0.
>> >
>> > The fix is simply to use signed type for the shift, and shift to
>> > the opposite direction for negative values.
>>
>> Series
>> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> Thanks for reviewing this.
>
>> I guess we don't have very many devices that are BE and
>> that allow the guest to access them with a smaller
>> width than their implemented width...
>
> Do you think is it worth using the '(un)likely()' macros?

Probably not; that was just a comment on why we haven't run
into the bug sooner.

thanks
-- PMM