Re: [Minios-devel] [PATCH v3 10/43] arm64: add exception support

[Julien Grall <julien.grall@xxxxxxx>]

Hi Shijie,

On 02/05/18 10:20, Huang Shijie wrote:
> On Mon, Apr 30, 2018 at 09:05:23AM +0100, Julien Grall wrote:
> > On 28/04/2018 06:40, Huang Shijie wrote:
> > > On Wed, Apr 18, 2018 at 06:48:24PM +0100, Julien Grall wrote:
> > > > > +       mrs     x18, sp_el0
> > > > > +.endif
> > > > Hmm I think you want the "mov x18, sp" but you also need to add the
> > > > PT_REG_SIZE to get the correct sp.
> > > The sp is correct here.
> > > We have sub the PT_REG_SIZE at the beginning of the @save_registers.
> > Well, the SP you want to store is the stack pointer before the context was
> > saved. This is the more meaningful one when you want to print the context
> > what was running before the exception.
> >
> > But as you will need to move the instruction mov x18, sp here you will end
> > to up to store the wrong SP here.
> Do you mean I should store the SP like this:
>
>         --------------------------------------
>         sub     sp, sp, #(PT_REG_SIZE)
>
>         mov     x18, sp
>
>         stp     ....
>         stp     ....
>         stp     ....
>         --------------------------------------
>    
> In this way, we store the correct SP value.
I don't understand your suggestion. Can you expand it?

[...]

> > > > > +        eret
> > > > > +.endm
> > > > > +
> > > > > +    .globl IRQ_handler
> > > > > +IRQ_handler:
> > > > > +    .long 0x0
> > > > I am not sure to understand the purpose of IRQ_handler. Can't you just
> > > > directly call the handler?
> > > We have already have the el1_irq to handle the IRQ.
> > > and we need to save the context before call IRQ_handler.
> > You didn't understand why question. I didn't ask whether you can replace
> > el1_irq by IRQ_handler, but why you need a pointer to the handler here
> > rather than calling gic_handler in el1_irq directly.
> Yes, I think we can call gic_handler directly in el1_irq.
>
> I admit that I did not do the code optimization for it.
It is not really about optimization. It is more about code that does not 
have any justification...
> > [...]
> >
> > > > > +#define PT_REG_SIZE   (272)
> > > > > +
> > > > > +#define PT_REG_SP     (0)
> > > > > +#define PT_REG_ELR    (16)
> > > > > +#define PT_REG_SPSR   (24)
> > > > > +#define PT_REG_X      (32)
> > > > I honestly don't like hardcoding offset of the structure. This is a real
> > > > call to mess up in the future (or even now). The lack of comments don't help
> > > > either.
> > > >
> > > > But I am pretty sure I asked it before. Can't they be generated
> > > > automatically?
> > > I do not know how to generated it automatically :(
> > I have already suggested way in the previous e-mails. I am sure you can go
> > through them again and look at what was discussed.
> sorry, I searched my old email, but I cannot find it.
> could you please tell me again?
https://lists.xenproject.org/archives/html/minios-devel/2017-11/msg00052.html

Cheers,

--
Julien Grall

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel