From patchwork Thu Mar 7 15:37:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 13585914 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7CA17C54798 for ; Thu, 7 Mar 2024 15:38:11 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1riFon-0001xK-QJ; Thu, 07 Mar 2024 10:38:02 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1riFok-0001tL-0c for qemu-devel@nongnu.org; Thu, 07 Mar 2024 10:37:59 -0500 Received: from frasgout.his.huawei.com ([185.176.79.56]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1riFoT-0001Xh-45 for qemu-devel@nongnu.org; Thu, 07 Mar 2024 10:37:56 -0500 Received: from mail.maildlp.com (unknown [172.18.186.216]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4TrCxw6qWMz6K7GH; Thu, 7 Mar 2024 23:33:40 +0800 (CST) Received: from lhrpeml500005.china.huawei.com (unknown [7.191.163.240]) by mail.maildlp.com (Postfix) with ESMTPS id 31A0A140B18; Thu, 7 Mar 2024 23:37:39 +0800 (CST) Received: from SecurePC-101-06.china.huawei.com (10.122.247.231) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Thu, 7 Mar 2024 15:37:38 +0000 To: Paolo Bonzini , Peter Xu , David Hildenbrand , =?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= , CC: Subject: [PATCH v2 1/4] physmem: Rename addr1 to more informative mr_addr in flatview_read/write() and similar Date: Thu, 7 Mar 2024 15:37:07 +0000 Message-ID: <20240307153710.30907-2-Jonathan.Cameron@huawei.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240307153710.30907-1-Jonathan.Cameron@huawei.com> References: <20240307153710.30907-1-Jonathan.Cameron@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.122.247.231] X-ClientProxiedBy: lhrpeml500001.china.huawei.com (7.191.163.213) To lhrpeml500005.china.huawei.com (7.191.163.240) Received-SPF: pass client-ip=185.176.79.56; envelope-from=jonathan.cameron@huawei.com; helo=frasgout.his.huawei.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-to: Jonathan Cameron X-Patchwork-Original-From: Jonathan Cameron via From: Jonathan Cameron Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org The calls to flatview_read/write[_continue]() have parameters addr and addr1 but the names give no indication of what they are addresses of. Rename addr1 to mr_addr to reflect that it is the translated address offset within the MemoryRegion returned by flatview_translate(). Similarly rename the parameter in address_space_read/write_cached_slow() Suggested-by: Peter Xu Signed-off-by: Jonathan Cameron Reviewed-by: David Hildenbrand --- v2: New patch. - I have kept the renames to only the code I'm touching later in this series, but they could be applied much more widely. --- system/physmem.c | 50 ++++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/system/physmem.c b/system/physmem.c index 05997a7ca7..2704b780f6 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -2685,7 +2685,7 @@ static bool flatview_access_allowed(MemoryRegion *mr, MemTxAttrs attrs, static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr, MemTxAttrs attrs, const void *ptr, - hwaddr len, hwaddr addr1, + hwaddr len, hwaddr mr_addr, hwaddr l, MemoryRegion *mr) { uint8_t *ram_ptr; @@ -2695,12 +2695,12 @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr, const uint8_t *buf = ptr; for (;;) { - if (!flatview_access_allowed(mr, attrs, addr1, l)) { + if (!flatview_access_allowed(mr, attrs, mr_addr, l)) { result |= MEMTX_ACCESS_ERROR; /* Keep going. */ } else if (!memory_access_is_direct(mr, true)) { release_lock |= prepare_mmio_access(mr); - l = memory_access_size(mr, l, addr1); + l = memory_access_size(mr, l, mr_addr); /* XXX: could force current_cpu to NULL to avoid potential bugs */ @@ -2715,13 +2715,13 @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr, (l == 8 && len >= 8)); #endif val = ldn_he_p(buf, l); - result |= memory_region_dispatch_write(mr, addr1, val, + result |= memory_region_dispatch_write(mr, mr_addr, val, size_memop(l), attrs); } else { /* RAM case */ - ram_ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false); + ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, &l, false); memmove(ram_ptr, buf, l); - invalidate_and_set_dirty(mr, addr1, l); + invalidate_and_set_dirty(mr, mr_addr, l); } if (release_lock) { @@ -2738,7 +2738,7 @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr, } l = len; - mr = flatview_translate(fv, addr, &addr1, &l, true, attrs); + mr = flatview_translate(fv, addr, &mr_addr, &l, true, attrs); } return result; @@ -2749,22 +2749,22 @@ static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs, const void *buf, hwaddr len) { hwaddr l; - hwaddr addr1; + hwaddr mr_addr; MemoryRegion *mr; l = len; - mr = flatview_translate(fv, addr, &addr1, &l, true, attrs); + mr = flatview_translate(fv, addr, &mr_addr, &l, true, attrs); if (!flatview_access_allowed(mr, attrs, addr, len)) { return MEMTX_ACCESS_ERROR; } return flatview_write_continue(fv, addr, attrs, buf, len, - addr1, l, mr); + mr_addr, l, mr); } /* Called within RCU critical section. */ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, MemTxAttrs attrs, void *ptr, - hwaddr len, hwaddr addr1, hwaddr l, + hwaddr len, hwaddr mr_addr, hwaddr l, MemoryRegion *mr) { uint8_t *ram_ptr; @@ -2775,14 +2775,14 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, fuzz_dma_read_cb(addr, len, mr); for (;;) { - if (!flatview_access_allowed(mr, attrs, addr1, l)) { + if (!flatview_access_allowed(mr, attrs, mr_addr, l)) { result |= MEMTX_ACCESS_ERROR; /* Keep going. */ } else if (!memory_access_is_direct(mr, false)) { /* I/O case */ release_lock |= prepare_mmio_access(mr); - l = memory_access_size(mr, l, addr1); - result |= memory_region_dispatch_read(mr, addr1, &val, + l = memory_access_size(mr, l, mr_addr); + result |= memory_region_dispatch_read(mr, mr_addr, &val, size_memop(l), attrs); /* @@ -2798,7 +2798,7 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, stn_he_p(buf, l, val); } else { /* RAM case */ - ram_ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false); + ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, &l, false); memcpy(buf, ram_ptr, l); } @@ -2816,7 +2816,7 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, } l = len; - mr = flatview_translate(fv, addr, &addr1, &l, false, attrs); + mr = flatview_translate(fv, addr, &mr_addr, &l, false, attrs); } return result; @@ -2827,16 +2827,16 @@ static MemTxResult flatview_read(FlatView *fv, hwaddr addr, MemTxAttrs attrs, void *buf, hwaddr len) { hwaddr l; - hwaddr addr1; + hwaddr mr_addr; MemoryRegion *mr; l = len; - mr = flatview_translate(fv, addr, &addr1, &l, false, attrs); + mr = flatview_translate(fv, addr, &mr_addr, &l, false, attrs); if (!flatview_access_allowed(mr, attrs, addr, len)) { return MEMTX_ACCESS_ERROR; } return flatview_read_continue(fv, addr, attrs, buf, len, - addr1, l, mr); + mr_addr, l, mr); } MemTxResult address_space_read_full(AddressSpace *as, hwaddr addr, @@ -3359,15 +3359,15 @@ MemTxResult address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr, void *buf, hwaddr len) { - hwaddr addr1, l; + hwaddr mr_addr, l; MemoryRegion *mr; l = len; - mr = address_space_translate_cached(cache, addr, &addr1, &l, false, + mr = address_space_translate_cached(cache, addr, &mr_addr, &l, false, MEMTXATTRS_UNSPECIFIED); return flatview_read_continue(cache->fv, addr, MEMTXATTRS_UNSPECIFIED, buf, len, - addr1, l, mr); + mr_addr, l, mr); } /* Called from RCU critical section. address_space_write_cached uses this @@ -3377,15 +3377,15 @@ MemTxResult address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr, const void *buf, hwaddr len) { - hwaddr addr1, l; + hwaddr mr_addr, l; MemoryRegion *mr; l = len; - mr = address_space_translate_cached(cache, addr, &addr1, &l, true, + mr = address_space_translate_cached(cache, addr, &mr_addr, &l, true, MEMTXATTRS_UNSPECIFIED); return flatview_write_continue(cache->fv, addr, MEMTXATTRS_UNSPECIFIED, buf, len, - addr1, l, mr); + mr_addr, l, mr); } #define ARG1_DECL MemoryRegionCache *cache From patchwork Thu Mar 7 15:37:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 13585915 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 107B0C54798 for ; Thu, 7 Mar 2024 15:38:36 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1riFp0-0002GF-EN; Thu, 07 Mar 2024 10:38:14 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1riFoz-0002Dq-G1 for qemu-devel@nongnu.org; Thu, 07 Mar 2024 10:38:13 -0500 Received: from frasgout.his.huawei.com ([185.176.79.56]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1riFox-0001kP-F6 for qemu-devel@nongnu.org; Thu, 07 Mar 2024 10:38:13 -0500 Received: from mail.maildlp.com (unknown [172.18.186.216]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4TrD3227kmz6D8dd; Thu, 7 Mar 2024 23:38:06 +0800 (CST) Received: from lhrpeml500005.china.huawei.com (unknown [7.191.163.240]) by mail.maildlp.com (Postfix) with ESMTPS id 9C809140736; Thu, 7 Mar 2024 23:38:09 +0800 (CST) Received: from SecurePC-101-06.china.huawei.com (10.122.247.231) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Thu, 7 Mar 2024 15:38:09 +0000 To: Paolo Bonzini , Peter Xu , David Hildenbrand , =?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= , CC: Subject: [PATCH v2 2/4] physmem: Reduce local variable scope in flatview_read/write_continue() Date: Thu, 7 Mar 2024 15:37:08 +0000 Message-ID: <20240307153710.30907-3-Jonathan.Cameron@huawei.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240307153710.30907-1-Jonathan.Cameron@huawei.com> References: <20240307153710.30907-1-Jonathan.Cameron@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.122.247.231] X-ClientProxiedBy: lhrpeml500001.china.huawei.com (7.191.163.213) To lhrpeml500005.china.huawei.com (7.191.163.240) Received-SPF: pass client-ip=185.176.79.56; envelope-from=jonathan.cameron@huawei.com; helo=frasgout.his.huawei.com X-Spam_score_int: -41 X-Spam_score: -4.2 X-Spam_bar: ---- X-Spam_report: (-4.2 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-to: Jonathan Cameron X-Patchwork-Original-From: Jonathan Cameron via From: Jonathan Cameron Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Precursor to factoring out the inner loops for reuse. Reviewed-by: Peter Xu Signed-off-by: Jonathan Cameron Reviewed-by: David Hildenbrand Reviewed-by: Philippe Mathieu-Daudé --- v2: Picked up tag from Peter. system/physmem.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/system/physmem.c b/system/physmem.c index 2704b780f6..a64a96a3e5 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -2688,10 +2688,7 @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr, hwaddr len, hwaddr mr_addr, hwaddr l, MemoryRegion *mr) { - uint8_t *ram_ptr; - uint64_t val; MemTxResult result = MEMTX_OK; - bool release_lock = false; const uint8_t *buf = ptr; for (;;) { @@ -2699,7 +2696,9 @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr, result |= MEMTX_ACCESS_ERROR; /* Keep going. */ } else if (!memory_access_is_direct(mr, true)) { - release_lock |= prepare_mmio_access(mr); + uint64_t val; + bool release_lock = prepare_mmio_access(mr); + l = memory_access_size(mr, l, mr_addr); /* XXX: could force current_cpu to NULL to avoid potential bugs */ @@ -2717,18 +2716,21 @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr, val = ldn_he_p(buf, l); result |= memory_region_dispatch_write(mr, mr_addr, val, size_memop(l), attrs); + if (release_lock) { + bql_unlock(); + } + + } else { /* RAM case */ - ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, &l, false); + + uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, &l, + false); + memmove(ram_ptr, buf, l); invalidate_and_set_dirty(mr, mr_addr, l); } - if (release_lock) { - bql_unlock(); - release_lock = false; - } - len -= l; buf += l; addr += l; @@ -2767,10 +2769,7 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, hwaddr len, hwaddr mr_addr, hwaddr l, MemoryRegion *mr) { - uint8_t *ram_ptr; - uint64_t val; MemTxResult result = MEMTX_OK; - bool release_lock = false; uint8_t *buf = ptr; fuzz_dma_read_cb(addr, len, mr); @@ -2780,7 +2779,9 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, /* Keep going. */ } else if (!memory_access_is_direct(mr, false)) { /* I/O case */ - release_lock |= prepare_mmio_access(mr); + uint64_t val; + bool release_lock = prepare_mmio_access(mr); + l = memory_access_size(mr, l, mr_addr); result |= memory_region_dispatch_read(mr, mr_addr, &val, size_memop(l), attrs); @@ -2796,17 +2797,16 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, (l == 8 && len >= 8)); #endif stn_he_p(buf, l, val); + if (release_lock) { + bql_unlock(); + } } else { /* RAM case */ - ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, &l, false); + uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, &l, + false); memcpy(buf, ram_ptr, l); } - if (release_lock) { - bql_unlock(); - release_lock = false; - } - len -= l; buf += l; addr += l; From patchwork Thu Mar 7 15:37:09 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 13585916 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EC083C54798 for ; Thu, 7 Mar 2024 15:38:55 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1riFpX-0003iW-Pm; Thu, 07 Mar 2024 10:38:47 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1riFpV-0003hO-GN for qemu-devel@nongnu.org; Thu, 07 Mar 2024 10:38:45 -0500 Received: from frasgout.his.huawei.com ([185.176.79.56]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1riFpT-0001uV-0l for qemu-devel@nongnu.org; Thu, 07 Mar 2024 10:38:45 -0500 Received: from mail.maildlp.com (unknown [172.18.186.231]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4TrCy56q9Xz6K5nJ; Thu, 7 Mar 2024 23:33:49 +0800 (CST) Received: from lhrpeml500005.china.huawei.com (unknown [7.191.163.240]) by mail.maildlp.com (Postfix) with ESMTPS id 16F5C14058E; Thu, 7 Mar 2024 23:38:40 +0800 (CST) Received: from SecurePC-101-06.china.huawei.com (10.122.247.231) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Thu, 7 Mar 2024 15:38:39 +0000 To: Paolo Bonzini , Peter Xu , David Hildenbrand , =?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= , CC: Subject: [PATCH v2 3/4] physmem: Factor out body of flatview_read/write_continue() loop Date: Thu, 7 Mar 2024 15:37:09 +0000 Message-ID: <20240307153710.30907-4-Jonathan.Cameron@huawei.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240307153710.30907-1-Jonathan.Cameron@huawei.com> References: <20240307153710.30907-1-Jonathan.Cameron@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.122.247.231] X-ClientProxiedBy: lhrpeml500001.china.huawei.com (7.191.163.213) To lhrpeml500005.china.huawei.com (7.191.163.240) Received-SPF: pass client-ip=185.176.79.56; envelope-from=jonathan.cameron@huawei.com; helo=frasgout.his.huawei.com X-Spam_score_int: -41 X-Spam_score: -4.2 X-Spam_bar: ---- X-Spam_report: (-4.2 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-to: Jonathan Cameron X-Patchwork-Original-From: Jonathan Cameron via From: Jonathan Cameron Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org This code will be reused for the address_space_cached accessors shortly. Also reduce scope of result variable now we aren't directly calling this in the loop. Signed-off-by: Jonathan Cameron Reviewed-by: David Hildenbrand --- v2: Thanks to Peter Xu - Fix alignment of code. - Drop unused addr parameter. - Carry through new mr_addr parameter name. - RB not picked up as not sure what Peter will think wrt to resulting parameter ordering. --- system/physmem.c | 169 +++++++++++++++++++++++++++-------------------- 1 file changed, 99 insertions(+), 70 deletions(-) diff --git a/system/physmem.c b/system/physmem.c index a64a96a3e5..1264eab24b 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -2681,6 +2681,56 @@ static bool flatview_access_allowed(MemoryRegion *mr, MemTxAttrs attrs, return false; } +static MemTxResult flatview_write_continue_step(MemTxAttrs attrs, + const uint8_t *buf, + hwaddr len, hwaddr mr_addr, + hwaddr *l, MemoryRegion *mr) +{ + if (!flatview_access_allowed(mr, attrs, mr_addr, *l)) { + return MEMTX_ACCESS_ERROR; + } + + if (!memory_access_is_direct(mr, true)) { + uint64_t val; + MemTxResult result; + bool release_lock = prepare_mmio_access(mr); + + *l = memory_access_size(mr, *l, mr_addr); + /* + * XXX: could force current_cpu to NULL to avoid + * potential bugs + */ + + /* + * Assure Coverity (and ourselves) that we are not going to OVERRUN + * the buffer by following ldn_he_p(). + */ +#ifdef QEMU_STATIC_ANALYSIS + assert((*l == 1 && len >= 1) || + (*l == 2 && len >= 2) || + (*l == 4 && len >= 4) || + (*l == 8 && len >= 8)); +#endif + val = ldn_he_p(buf, *l); + result = memory_region_dispatch_write(mr, mr_addr, val, + size_memop(*l), attrs); + if (release_lock) { + bql_unlock(); + } + + return result; + } else { + /* RAM case */ + uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l, + false); + + memmove(ram_ptr, buf, *l); + invalidate_and_set_dirty(mr, mr_addr, *l); + + return MEMTX_OK; + } +} + /* Called within RCU critical section. */ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr, MemTxAttrs attrs, @@ -2692,44 +2742,8 @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr, const uint8_t *buf = ptr; for (;;) { - if (!flatview_access_allowed(mr, attrs, mr_addr, l)) { - result |= MEMTX_ACCESS_ERROR; - /* Keep going. */ - } else if (!memory_access_is_direct(mr, true)) { - uint64_t val; - bool release_lock = prepare_mmio_access(mr); - - l = memory_access_size(mr, l, mr_addr); - /* XXX: could force current_cpu to NULL to avoid - potential bugs */ - - /* - * Assure Coverity (and ourselves) that we are not going to OVERRUN - * the buffer by following ldn_he_p(). - */ -#ifdef QEMU_STATIC_ANALYSIS - assert((l == 1 && len >= 1) || - (l == 2 && len >= 2) || - (l == 4 && len >= 4) || - (l == 8 && len >= 8)); -#endif - val = ldn_he_p(buf, l); - result |= memory_region_dispatch_write(mr, mr_addr, val, - size_memop(l), attrs); - if (release_lock) { - bql_unlock(); - } - - - } else { - /* RAM case */ - - uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, &l, - false); - - memmove(ram_ptr, buf, l); - invalidate_and_set_dirty(mr, mr_addr, l); - } + result |= flatview_write_continue_step(attrs, buf, len, mr_addr, &l, + mr); len -= l; buf += l; @@ -2763,6 +2777,52 @@ static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs, mr_addr, l, mr); } +static MemTxResult flatview_read_continue_step(MemTxAttrs attrs, uint8_t *buf, + hwaddr len, hwaddr mr_addr, + hwaddr *l, + MemoryRegion *mr) +{ + if (!flatview_access_allowed(mr, attrs, mr_addr, *l)) { + return MEMTX_ACCESS_ERROR; + } + + if (!memory_access_is_direct(mr, false)) { + /* I/O case */ + uint64_t val; + MemTxResult result; + bool release_lock = prepare_mmio_access(mr); + + *l = memory_access_size(mr, *l, mr_addr); + result = memory_region_dispatch_read(mr, mr_addr, &val, size_memop(*l), + attrs); + + /* + * Assure Coverity (and ourselves) that we are not going to OVERRUN + * the buffer by following stn_he_p(). + */ +#ifdef QEMU_STATIC_ANALYSIS + assert((*l == 1 && len >= 1) || + (*l == 2 && len >= 2) || + (*l == 4 && len >= 4) || + (*l == 8 && len >= 8)); +#endif + stn_he_p(buf, *l, val); + + if (release_lock) { + bql_unlock(); + } + return result; + } else { + /* RAM case */ + uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, l, + false); + + memcpy(buf, ram_ptr, *l); + + return MEMTX_OK; + } +} + /* Called within RCU critical section. */ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, MemTxAttrs attrs, void *ptr, @@ -2774,38 +2834,7 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, fuzz_dma_read_cb(addr, len, mr); for (;;) { - if (!flatview_access_allowed(mr, attrs, mr_addr, l)) { - result |= MEMTX_ACCESS_ERROR; - /* Keep going. */ - } else if (!memory_access_is_direct(mr, false)) { - /* I/O case */ - uint64_t val; - bool release_lock = prepare_mmio_access(mr); - - l = memory_access_size(mr, l, mr_addr); - result |= memory_region_dispatch_read(mr, mr_addr, &val, - size_memop(l), attrs); - - /* - * Assure Coverity (and ourselves) that we are not going to OVERRUN - * the buffer by following stn_he_p(). - */ -#ifdef QEMU_STATIC_ANALYSIS - assert((l == 1 && len >= 1) || - (l == 2 && len >= 2) || - (l == 4 && len >= 4) || - (l == 8 && len >= 8)); -#endif - stn_he_p(buf, l, val); - if (release_lock) { - bql_unlock(); - } - } else { - /* RAM case */ - uint8_t *ram_ptr = qemu_ram_ptr_length(mr->ram_block, mr_addr, &l, - false); - memcpy(buf, ram_ptr, l); - } + result |= flatview_read_continue_step(attrs, buf, len, mr_addr, &l, mr); len -= l; buf += l; From patchwork Thu Mar 7 15:37:10 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Cameron X-Patchwork-Id: 13585917 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 690E0C54E49 for ; Thu, 7 Mar 2024 15:39:39 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1riFq1-0004wO-Du; Thu, 07 Mar 2024 10:39:17 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1riFpy-0004sm-FG for qemu-devel@nongnu.org; Thu, 07 Mar 2024 10:39:14 -0500 Received: from frasgout.his.huawei.com ([185.176.79.56]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1riFpw-000259-8K for qemu-devel@nongnu.org; Thu, 07 Mar 2024 10:39:14 -0500 Received: from mail.maildlp.com (unknown [172.18.186.31]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4TrD4C2VsMz6D8bw; Thu, 7 Mar 2024 23:39:07 +0800 (CST) Received: from lhrpeml500005.china.huawei.com (unknown [7.191.163.240]) by mail.maildlp.com (Postfix) with ESMTPS id A334C140DF4; Thu, 7 Mar 2024 23:39:10 +0800 (CST) Received: from SecurePC-101-06.china.huawei.com (10.122.247.231) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.35; Thu, 7 Mar 2024 15:39:10 +0000 To: Paolo Bonzini , Peter Xu , David Hildenbrand , =?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= , CC: Subject: [PATCH v2 4/4] physmem: Fix wrong address in large address_space_read/write_cached_slow() Date: Thu, 7 Mar 2024 15:37:10 +0000 Message-ID: <20240307153710.30907-5-Jonathan.Cameron@huawei.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240307153710.30907-1-Jonathan.Cameron@huawei.com> References: <20240307153710.30907-1-Jonathan.Cameron@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.122.247.231] X-ClientProxiedBy: lhrpeml500001.china.huawei.com (7.191.163.213) To lhrpeml500005.china.huawei.com (7.191.163.240) Received-SPF: pass client-ip=185.176.79.56; envelope-from=jonathan.cameron@huawei.com; helo=frasgout.his.huawei.com X-Spam_score_int: -41 X-Spam_score: -4.2 X-Spam_bar: ---- X-Spam_report: (-4.2 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-to: Jonathan Cameron X-Patchwork-Original-From: Jonathan Cameron via From: Jonathan Cameron Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org If the access is bigger than the MemoryRegion supports, flatview_read/write_continue() will attempt to update the Memory Region. but the address passed to flatview_translate() is relative to the cache, not to the FlatView. On arm/virt with interleaved CXL memory emulation and virtio-blk-pci this lead to the first part of descriptor being read from the CXL memory and the second part from PA 0x8 which happens to be a blank region of a flash chip and all ffs on this particular configuration. Note this test requires the out of tree ARM support for CXL, but the problem is more general. Avoid this by adding new address_space_read_continue_cached() and address_space_write_continue_cached() which share all the logic with the flatview versions except for the MemoryRegion lookup which is unnecessary as the MemoryRegionCache only covers one MemoryRegion. Signed-off-by: Jonathan Cameron Reviewed-by: David Hildenbrand --- v2: Review from Peter Xu - Drop additional lookups of the MemoryRegion via address_space_translate_cached() as it will always return the same answer. - Drop various parameters that are then unused. - rename addr1 to mr_addr. - Drop a fuzz_dma_read_cb(). Could put this back but it means carrying the address into the inner call and the only in tree fuzzer checks if it is normal RAM and if not does nothing anyway. We don't hit this path for normal RAM. --- system/physmem.c | 63 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 6 deletions(-) diff --git a/system/physmem.c b/system/physmem.c index 1264eab24b..701bea27dd 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -3381,6 +3381,59 @@ static inline MemoryRegion *address_space_translate_cached( return section.mr; } +/* Called within RCU critical section. */ +static MemTxResult address_space_write_continue_cached(MemTxAttrs attrs, + const void *ptr, + hwaddr len, + hwaddr mr_addr, + hwaddr l, + MemoryRegion *mr) +{ + MemTxResult result = MEMTX_OK; + const uint8_t *buf = ptr; + + for (;;) { + result |= flatview_write_continue_step(attrs, buf, len, mr_addr, &l, + mr); + + len -= l; + buf += l; + mr_addr += l; + + if (!len) { + break; + } + + l = len; + } + + return result; +} + +/* Called within RCU critical section. */ +static MemTxResult address_space_read_continue_cached(MemTxAttrs attrs, + void *ptr, hwaddr len, + hwaddr mr_addr, hwaddr l, + MemoryRegion *mr) +{ + MemTxResult result = MEMTX_OK; + uint8_t *buf = ptr; + + for (;;) { + result |= flatview_read_continue_step(attrs, buf, len, mr_addr, &l, mr); + len -= l; + buf += l; + mr_addr += l; + + if (!len) { + break; + } + l = len; + } + + return result; +} + /* Called from RCU critical section. address_space_read_cached uses this * out of line function when the target is an MMIO or IOMMU region. */ @@ -3394,9 +3447,8 @@ address_space_read_cached_slow(MemoryRegionCache *cache, hwaddr addr, l = len; mr = address_space_translate_cached(cache, addr, &mr_addr, &l, false, MEMTXATTRS_UNSPECIFIED); - return flatview_read_continue(cache->fv, - addr, MEMTXATTRS_UNSPECIFIED, buf, len, - mr_addr, l, mr); + return address_space_read_continue_cached(MEMTXATTRS_UNSPECIFIED, + buf, len, mr_addr, l, mr); } /* Called from RCU critical section. address_space_write_cached uses this @@ -3412,9 +3464,8 @@ address_space_write_cached_slow(MemoryRegionCache *cache, hwaddr addr, l = len; mr = address_space_translate_cached(cache, addr, &mr_addr, &l, true, MEMTXATTRS_UNSPECIFIED); - return flatview_write_continue(cache->fv, - addr, MEMTXATTRS_UNSPECIFIED, buf, len, - mr_addr, l, mr); + return address_space_write_continue_cached(MEMTXATTRS_UNSPECIFIED, + buf, len, mr_addr, l, mr); } #define ARG1_DECL MemoryRegionCache *cache