diff mbox

[Xen-devel,26/34] xen/arm: traps: Drop dump_guest_s1_walk

Message ID 1395766541-23979-27-git-send-email-julien.grall@linaro.org
State Deferred, archived
Headers show

Commit Message

Julien Grall March 25, 2014, 4:55 p.m. UTC
This function is not used neither export.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
Cc: Tim Deegan <tim@xen.org>
---
 xen/arch/arm/traps.c |   56 --------------------------------------------------
 1 file changed, 56 deletions(-)

Comments

Ian Campbell March 27, 2014, 5:09 p.m. UTC | #1
On Tue, 2014-03-25 at 16:55 +0000, Julien Grall wrote:
> This function is not used neither export.

I have used it when debugging stuff, where it is very useful.

Should it not be called from one of the dump_vcpu_state in response to
an exception which implies a guest translation fault?
Julien Grall April 1, 2014, 4:59 p.m. UTC | #2
On 03/27/2014 05:09 PM, Ian Campbell wrote:
> On Tue, 2014-03-25 at 16:55 +0000, Julien Grall wrote:
>> This function is not used neither export.
> 
> I have used it when debugging stuff, where it is very useful.

I was not sure if anyone was using it. I can export it in p2m.h.

I'm also thinking to move this function in p2m.c (which is a best place
for this function). What do you think?

> Should it not be called from one of the dump_vcpu_state in response to
> an exception which implies a guest translation fault?
> 

No, we inject directly an exception to the guest since few months. So we
don't have to print the p2m.

Regards,
Ian Campbell April 2, 2014, 8:38 a.m. UTC | #3
On Tue, 2014-04-01 at 17:59 +0100, Julien Grall wrote:
> On 03/27/2014 05:09 PM, Ian Campbell wrote:
> > On Tue, 2014-03-25 at 16:55 +0000, Julien Grall wrote:
> >> This function is not used neither export.
> > 
> > I have used it when debugging stuff, where it is very useful.
> 
> I was not sure if anyone was using it. I can export it in p2m.h.
> 
> I'm also thinking to move this function in p2m.c (which is a best place
> for this function). What do you think?

It's not really a p2m related function, it's guest stage 1. There isn't
really a good home for it I don't think. I suppose it has similarities
with dump_p2m_lookup/dump_phy_walk and therefore with the core
dump_pt_walk, which would then suggest mm.c as a good home for it.

> > Should it not be called from one of the dump_vcpu_state in response to
> > an exception which implies a guest translation fault?
> > 
> 
> No, we inject directly an exception to the guest since few months. So we
> don't have to print the p2m.

Ah, that's probably when the last caller disappeared.

On second thoughts, when debugging I can always rescue the code from the
git history, so lets go with your first instinct and remove it. The
original patch is:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.
diff mbox

Patch

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 2109d03..b1475f3 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1422,62 +1422,6 @@  static void do_sysreg(struct cpu_user_regs *regs,
 }
 #endif
 
-void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
-{
-    register_t ttbcr = READ_SYSREG(TCR_EL1);
-    uint64_t ttbr0 = READ_SYSREG64(TTBR0_EL1);
-    paddr_t paddr;
-    uint32_t offset;
-    uint32_t *first = NULL, *second = NULL;
-
-    printk("dom%d VA 0x%08"PRIvaddr"\n", d->domain_id, addr);
-    printk("    TTBCR: 0x%08"PRIregister"\n", ttbcr);
-    printk("    TTBR0: 0x%016"PRIx64" = 0x%"PRIpaddr"\n",
-           ttbr0, p2m_lookup(d, ttbr0 & PAGE_MASK, NULL));
-
-    if ( ttbcr & TTBCR_EAE )
-    {
-        printk("Cannot handle LPAE guest PT walk\n");
-        return;
-    }
-    if ( (ttbcr & TTBCR_N_MASK) != 0 )
-    {
-        printk("Cannot handle TTBR1 guest walks\n");
-        return;
-    }
-
-    paddr = p2m_lookup(d, ttbr0 & PAGE_MASK, NULL);
-    if ( paddr == INVALID_PADDR )
-    {
-        printk("Failed TTBR0 maddr lookup\n");
-        goto done;
-    }
-    first = map_domain_page(paddr>>PAGE_SHIFT);
-
-    offset = addr >> (12+10);
-    printk("1ST[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
-           offset, paddr, first[offset]);
-    if ( !(first[offset] & 0x1) ||
-         !(first[offset] & 0x2) )
-        goto done;
-
-    paddr = p2m_lookup(d, first[offset] & PAGE_MASK, NULL);
-
-    if ( paddr == INVALID_PADDR )
-    {
-        printk("Failed L1 entry maddr lookup\n");
-        goto done;
-    }
-    second = map_domain_page(paddr>>PAGE_SHIFT);
-    offset = (addr >> 12) & 0x3FF;
-    printk("2ND[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
-           offset, paddr, second[offset]);
-
-done:
-    if (second) unmap_domain_page(second);
-    if (first) unmap_domain_page(first);
-}
-
 static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
                                       union hsr hsr)
 {