Re: [PATCH v1] nrf51: Fix last GPIO CNF address

[Andrew Jeffery]

On Fri, 10 Apr 2020, at 21:56, Peter Maydell wrote:
> On Fri, 10 Apr 2020 at 04:42, Andrew Jeffery <address@hidden> wrote:
> > IIRC Phil wanted to enable sub-word accesses to the sample value
> > registers but I didn't want to spread logic dealing with different access
> > widths through the model. We already had a mechanism to describe the
> > model's  supported access sizes (as opposed to the valid access sizes
> > allowed by hardware) in the `impl` member of the MemoryRegionOps, so
> > I was trying to use that, but it didn't work as I needed.
> >
> > The accesses generated at the point of the guest MMIO need to be
> > expanded to the access width supported by the model and then the
> > resulting data trimmed again upon returning the data (in the case of a
> > read) via the MMIO operation.
> >
> > So the intent was less about unaligned accesses than enabling models
> > implementations that only have to handle certain-sized access widths.
> > It happens to help the unaligned access case as well.
> 
> Yeah, we definitely could do with improving things here, it is annoying
> to have to write code for handling some of the oddball cases when you
> have just one register that allows byte accesses or whatever.
> The thing I have in the back of my mind as a concern is that we have
> had several "is this a buffer overrun" questions where the answer has
> been "it can't be, because the core code doesn't allow a call to the
> read/write function for a 4 byte access where the address is not 4-aligned,
> so my_byte_array[addr] is always in-bounds".
> So if we change the core code we need to make sure we don't
> inadvertently remove a restriction that was protecting us from a guest
> escape...

Oh for sure. The patch was very RFC, as mentioned I just sent it to check
whether I was on the right track or off in the weeds, and from there it has
hung around in Cedric's tree without much progress.

Feels like the time is right to sort the problem out properly, which might
mean starting from scratch to make sure we don't miss any of the details.

Andrew