|
From: | David Hildenbrand |
Subject: | Re: [PATCH resend v2 1/5] tpm: mark correct memory region range dirty when clearing RAM |
Date: | Mon, 26 Jul 2021 10:08:59 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
On 24.07.21 00:35, Peter Xu wrote:
On Fri, Jul 23, 2021 at 09:15:43PM +0200, David Hildenbrand wrote:On 23.07.21 16:52, Peter Xu wrote:On Tue, Jul 20, 2021 at 03:03:00PM +0200, David Hildenbrand wrote:@@ -30,11 +30,13 @@ void tpm_ppi_reset(TPMPPI *tpmppi) guest_phys_blocks_init(&guest_phys_blocks); guest_phys_blocks_append(&guest_phys_blocks); QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) { + ram_addr_t mr_start = memory_region_get_ram_addr(block->mr); + trace_tpm_ppi_memset(block->host_addr, block->target_end - block->target_start); memset(block->host_addr, 0, block->target_end - block->target_start); - memory_region_set_dirty(block->mr, 0, + memory_region_set_dirty(block->mr, block->target_start - mr_start, block->target_end - block->target_start);target_start should falls in gpa range, while mr_start is ram_addr_t. I am not sure whether this is right..When I wrote that code I was under the impression that memory_region_get_ram_addr() would give the GPA where the memory region starts, but ... that's not correct as you point out. "offset" confusion :)Neither do I know how to get correct mr offset with the existing info we've got from GuestPhysBlock. Maybe we need to teach guest_phys_blocks_region_add() to also record section->offset_within_region?We might actually want offset_within_address_space + offset_within_region, so we can calculate the GPA difference to see where inside the ramblock we end up.I still think offset_within_region is exactly what we want to fill in here, but you can do a double check.
I remember when I first looked into that months ago I wanted to avoid extending GuestPhysBlock. The commit message actually tells us what to do, and where my optimization went wrong :) "We might not start at the beginning of the memory region. We could also calculate via the difference in the host address; however, memory_region_set_dirty() also relies on memory_region_get_ram_addr() internally, so let's just use that." So, avoiding the optimization, we'd be left with: diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c index 362edcc5c9..fab49524d7 100644 --- a/hw/tpm/tpm_ppi.c +++ b/hw/tpm/tpm_ppi.c @@ -30,11 +30,14 @@ void tpm_ppi_reset(TPMPPI *tpmppi) guest_phys_blocks_init(&guest_phys_blocks); guest_phys_blocks_append(&guest_phys_blocks); QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) { + hwaddr mr_offs = (uint8_t *) memory_region_get_ram_ptr(block->mr) - + block->host_addr; + trace_tpm_ppi_memset(block->host_addr, block->target_end - block->target_start); memset(block->host_addr, 0, block->target_end - block->target_start); - memory_region_set_dirty(block->mr, 0, + memory_region_set_dirty(block->mr, mr_offs, block->target_end - block->target_start); } guest_phys_blocks_free(&guest_phys_blocks); That should make more sense :) -- Thanks, David / dhildenb
[Prev in Thread] | Current Thread | [Next in Thread] |