Re: [PATCH v2 0/2] Drop ignore_memory_transaction_failures for xilink_zynq

[Chao Liu <chao.liu@yeah.net>]

[-- Attachment #1: Type: text/plain, Size: 2808 bytes --]

On 2024/9/27 20:18, Peter Maydell wrote:

> On Fri, 27 Sept 2024 at 09:52, Chao Liu<chao.liu@yeah.net> wrote:
>> Hi, thank you for your prompt reply, it's a great encouragement to me!
>>
>> Based on your review suggestions, I have improved the v1 patch.
>>
>> By using create_unimplemented_device() during the initialization phase,
>> I added a "znyq.umip" device early on, which covers the 32-bit address space
>> of GPA. This can better serve as a replacement for the effect of the
>> ignore_memory_transaction_failures flag.
>>
>> Since create_unimplemented_device() sets the priority of the
>> memory region (mr) to -100, normally created devices will override the address
>> segments corresponding to the unimplemented devices.
>>
>> Even if our test set is not sufficiently comprehensive, we can create an
>> unimp_device for the maximum address space allowed by the board. This prevents
>> the guest system from triggering unexpected exceptions when accessing
>> unimplemented devices or regions.
> What would be the benefit of doing that? If we're going to
> say "we'll make accesses to regions without devices not
> generate faults", the simplest way to do that is to
> leave the ignore_memory_transaction_failures flag set
> the way it is.
>
> thanks
> -- PMM

I noticed that the `ignore_memory_transaction_failures` flag
was introduced in ed860129ac ("boards.h: Define new flag
ignore_memory_transaction_failures")

This approach was wise given the circumstances at the time.

Initially, this flag was added to ensure compatibility with the
RAZ/WI behavior in the ARM legacy board model.

Currently, only the ARM legacy board model uses this flag.

Introducing this flag provides a straightforward way to suppress
memory access exceptions by checking if the flag is enabled after
a CPU memory access failure; however,its primary purpose is to
ensure compatibility.

The purpose was to ensure that the ARM legacy board model behaves
as expected under conditions where thorough testing was not feasible.

Since we can designate unimplemented device memory ranges with
"unimplemented-device," this represents a more standard approach in QEMU
for managing RAZ/WI behavior.

However, this approach requires some effort.

Consequently, I have prioritized the removal of the
ignore_memory_transaction_failures flag on the Xilinx Zynq board
and aim to replace it with a more general solution to enhance design
simplicity and consistency.

If my approach is approved, I am very glad to systematically remove the
ignore_memory_transaction_failures flag from other ARM legacy boards and
ultimately eliminate it from the MachineClass.

This is my first attempt at contributing patches to the QEMU community,
and there is much for me to learn, and thanks for your patience and efforts!

Best regards,
Chao Liu

[-- Attachment #2: Type: text/html, Size: 3357 bytes --]