Hi Peter,

On 08/17/2017 07:25 AM, Peter Maydell wrote:
On 5 August 2017 at 11:13, Peter Maydell <peter.mayd...@linaro.org> wrote:
On 4 August 2017 at 20:23, Richard Henderson
<richard.hender...@linaro.org> wrote:
On 08/04/2017 11:09 AM, Philippe Mathieu-Daudé wrote:
Since create_unimplemented_device() register overlapped with low priority, why
not register it as default device directly, over the whole address space?

That's a good suggestion.  It makes more sense to me than adding a flag on the
MachineClass.

Yeah, I did think about implementing it that way, but...

That wouldn't handle the case of a device model directly
returning a MEMTX_ERROR, or a transaction dispatched to
a memory region whose MemoryRegionOps valid settings
prohibit it (eg byte accesses to a word-access-only device),
or accesses to a MemoryRegion that was created by passing
a NULL MemoryRegionOps pointer to memory_region_init_io
(I dunno why you'd do that but some code does).

In short, there are lots of ways the memory subsystem might
end up returning a transaction error -- this mechanism
ensures that none of them start generating exceptions
when they previously did not, and is (I hope) easy to
review in the sense of being sure that it does what it
intends to do without the need to audit a lot of corner
cases.

So, this question (should we have a board flag to disable reporting
of tx failures to the CPU hook, or use unimplemented_device as a
sort of background region) seems to be the main unanswered question
for this series. I think (as outlined above) that the board flag
is simpler and safer; are people happy for me to put this series
in target-arm.next with that approach, or should I rethink this bit?
As remarked previously in this thread, the current QEMU behavior on transaction error isn't always matching real hardware.
Matching correctly throwing errors is likely to break various current users.

If we are worried about being backward compatible, defaulting background region to unimp() won't throw any transaction error.

Since the default is no transaction error, users who expect hardware error can implement the correct behavior, per region/bus transaction errors.

I'm somehow afraid that "ignore_memory_transaction_failures" ends up like the "cannot_instantiate_with_device_add_yet" flag - a hard to remove kludge outliving his purpose.

Anyway I'm not unhappy with this approach, but I'd be very happy to have unimp() covering the whole background region.

Regards,

Phil.

Reply via email to