Re: [Qemu-devel] [PULL 04/32] target/riscv: Implement riscv_cpu_unassigned_access

[Palmer Dabbelt <palmer@sifive.com>]

On Thu, 15 Aug 2019 14:39:18 PDT (-0700), alistair23@gmail.com wrote:
> On Tue, Aug 13, 2019 at 3:44 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>>
>> On Thu, 01 Aug 2019 08:39:17 PDT (-0700), Peter Maydell wrote:
>> > On Wed, 3 Jul 2019 at 09:41, Palmer Dabbelt <palmer@sifive.com> wrote:
>> >>
>> >> From: Michael Clark <mjc@sifive.com>
>> >>
>> >> This patch adds support for the riscv_cpu_unassigned_access call
>> >> and will raise a load or store access fault.
>> >>
>> >> Signed-off-by: Michael Clark <mjc@sifive.com>
>> >> [Changes by AF:
>> >>  - Squash two patches and rewrite commit message
>> >>  - Set baddr to the access address
>> >> ]
>> >> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> >> Reviewed-by: Palmer Dabbelt <palmer@sifive.com>
>> >> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
>> >
>> > Oops, I missed seeing this go by. The do_unassigned_access
>> > hook is deprecated and you should drop this and use
>> > the do_transaction_failed hook instead.
>
> Argh!
>
>> >
>> > The distinction between the two is that do_unassigned_access
>> > will end up being called for any failing access, including
>> > not just "normal" guest accesses but also for bad accesses
>> > that happen during page table walks (which often want to
>> > be reported to the guest differently) and also accesses
>> > by random devices like DMA controllers (where throwing a
>> > cpu exception is always a bug).
>> >
>> > Changing the hook implementation itself should be straightforward;
>> > commit 6ad4d7eed05a1e23537f is an example of doing that on Alpha.
>> > You also want to check all the places in your target code that
>> > do physical memory accesses, determine what the right behaviour
>> > if they get a bus fault is, and implement that (or at least put
>> > in TODO comments).
>>
>> Sorry, updating that has been on my TODO list for a while now.  I figured it
>> was better to have the deprecated version in there than nothing at all.  I've
>> written some patches to fix this, but I want to give them another look before
>> sending them out.
>
> I was going to start looking into this, but if you already have
> patches I won't bother. Let me know if you want a hand with the
> conversion.

You're more than welcome to take them over.  I've got something that boots 
Linux on my unassigned_access branch (github.com/palmer-dabbelt/qemu), but I 
haven't sanitized the whole port for physical accesses and I haven't convinced 
myself that my hook implementation is correct.