|
From: | Philippe Mathieu-Daudé |
Subject: | Re: [PATCH] Avoid cpu_physical_memory_rw() with a constant is_write argument |
Date: | Tue, 18 Feb 2020 21:07:18 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 |
On 2/18/20 7:49 PM, Peter Maydell wrote:
On Tue, 18 Feb 2020 at 17:57, Stefan Weil <address@hidden> wrote:Am 18.02.20 um 14:20 schrieb Philippe Mathieu-Daudé:This commit was produced with the included Coccinelle script scripts/coccinelle/as-rw-const.patch. Inspired-by: Peter Maydell <address@hidden> Signed-off-by: Philippe Mathieu-Daudé <address@hidden> --- Based-on: <address@hidden>[...]diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c index a8b6e5aeb8..f5971ccc74 100644 --- a/target/i386/hax-all.c +++ b/target/i386/hax-all.c @@ -376,8 +376,8 @@ static int hax_handle_fastmmio(CPUArchState *env, struct hax_fastmmio *hft) * hft->direction == 2: gpa ==> gpa2 */ uint64_t value; - cpu_physical_memory_rw(hft->gpa, (uint8_t *) &value, hft->size, 0); - cpu_physical_memory_rw(hft->gpa2, (uint8_t *) &value, hft->size, 1); + cpu_physical_memory_read(hft->gpa, (uint8_t *)&value, hft->size); + cpu_physical_memory_write(hft->gpa2, (uint8_t *)&value, hft->size);Maybe those type casts could be removed, too. They are no longer needed after your modification.I think that we should fix the inconsistency where these functions all take "uint8_t* buf": - address_space_rw() - address_space_read() - address_space_write() - address_space_write_rom() - cpu_physical_memory_rw() - cpu_memory_rw_debug() but these take void*: - cpu_physical_memory_read() - cpu_physical_memory_write() - address_space_write_cached() - address_space_read_cached_slow() - address_space_write_cached_slow() - pci_dma_read() - pci_dma_write() - pci_dma_rw() - dma_memory_read() - dma_memory_write() - dma_memory_rw() - dma_memory_rw_relaxed()
I don't understand well cpu_physical_memory*(). Aren't these obsolete? They confuse me when using multi-core CPUs.
Depending on which way we go we would either want to remove these casts, or not. I guess that we have more cases of 'void*', and that would certainly be the easier way to convert (otherwise we probably need to add a bunch of new casts to uint8_t* in various callsites), but I don't have a strong opinion. Paolo ?
I thought about it too but it is quite some work, and I'v to admit I lost some faith with my previous chardev conversion. There Paolo/Daniel agreed to follow the libc read()/write() prototypes.
thanks -- PMM
[Prev in Thread] | Current Thread | [Next in Thread] |