On 17 February 2016 at 19:09, Wei Huang <w...@redhat.com> wrote: > > > On 02/17/2016 11:53 AM, Peter Maydell wrote: >> On 17 February 2016 at 17:34, Wei Huang <w...@redhat.com> wrote: >>> On 02/16/2016 08:39 AM, Peter Maydell wrote: >>>> Side note: half our "PL061" behaviour is actually specific >>>> to the TI variant in the Luminary, and for our plain old PL061 >>>> we ought to restrict access to the registers that are Stellaris >>>> only. But that's a different bug and not a very major one. >>> >>> Thanks for your suggestion. I was trying to fix it. The plan was to add >>> a new field rsvd_addr in "struct PL061State". Then in pl061_read() and >>> pl061_write(), we can check offset against [rsvd_addr, 0xfcc] (ignored >>> if inside). >>> >>> While I was working on it, I realized that this is a benign issue. It is >>> true that PL061 device can access Luminary registers in the reserved >>> memory area. However QEMU doesn't use these Luminary registers anywhere >>> else other than pl061_read() and pl061_write(). It basically passes the >>> read/write requests through. I don't see a malicious driver can damage >>> device state. Thoughts? >> >> It's not a "malicious guest can do bad things" bug, it's a "modelled >> hardware doesn't behave like the real thing" bug. A non-Luminary PL061 >> should act like the hardware, which means that the registers that don't >> exist should be RAZ/WI (and should log guest-errors if the guest tries >> to access them), the same way we do in the "default" case of the >> case statements for other reserved registers. > > How about the attached patch? I can write a new patch based on it, or > you prefer stashing it on top of V3 I just submitted?
Looks OK, but you don't need to check for address being <= 0xfcc because either we've already handled the ID registers (read) or we were going to be reserved anyway (write). thanks -- PMM
