Hi, Kan,
Thanks for detail figure. From architecture correctness, I think
your patch is yes required though case 2 is only walked before dom0
loads rr7 (After that, vhpt table will be enabled from then on and case
1 is the only path then).
So please re-submit the patch again and it's better if you could
move the check to the point before late_alt_itlb_miss to avoid
duplicated check in case 2. Also, it's better to jump to page_fault
instead of crash the whole xen here which is over-killed. :-)
Thanks,
Kevin
>-----Original Message-----
>From: Masaki Kanno [mailto:kanno.masaki@xxxxxxxxxxxxxx]
>Sent: 2006年4月24日 15:40
>To: Tian, Kevin
>Cc: Alex Williamson; Isaku Yamahata; xen-ia64-devel
>Subject: Re: [Xen-ia64-devel] alt_itlb_miss?
>
>Hi Kevin,
>
>Thanks for your explanation.
>Sorry, I'd like you to explain this once again. Please look at the
>below figure.
>
>1) Instruction TLB Fault ---+
> |
> +----------------------+
> |
> +---> ENTRY(iltb_miss)
> /* Check ifa (It was VHPT_CCHAIN_LOOKUP before
>here) */
> mov r16 = cr.ifa
> extr.u r17=r16,59,5
> cmp.eq p6,p0=0x1e,r17
> (p6) br.cond.spnt late_alt_itlb_miss -----+
> cmp.eq p6,p0=0x1d,r17 |
> (p6) br.cond.spnt late_alt_itlb_miss ---+ |
> | |
> | |
>2) Alternate Instruction TLB Fault ---+ | |
> | | |
> +--------------------------------+ | |
> | | |
> +---> ENTRY(alt_itlb_miss) | |
> mov r16=cr.ifa | |
> | |
> late_alt_itlb_miss: <-------------------+-+
>
> /* Check cpl */
> cmp.ne p8,p0=r0,r23
> or r19=r17,r19
> or r19=r19,r18
> (p8) br.cond.spnt page_fault
>
> + /* Check ifa with my patch */
> + extr.u r22=r16,59,5
> + cmp.ne p8,p0=0x1e,r22
> + (p8) br.cond.spnt 1f ----------+
> |
> itc.i r19 |
> mov pr=r31,-1 |
> rfi |
> |
> + 1: <---------------------------+
> + FORCE_CRASH
>
>If case 1), I think that a FORCE_CRASH and ifa checking is
>unnecessary
>according to your explanation.
>If case 2), I think that a FORCE_CRASH and ifa checking is necessary.
>Because, I thought that Xen may use a wrong address.
>If case 2), does Xen trust only cpl?
>
>Best regards,
> Kan
>
>Tian, Kevin wrote:
>>>From: Masaki Kanno [mailto:kanno.masaki@xxxxxxxxxxxxxx]
>>>Sent: 2006定4??21?? 18:56
>>>>>
>>>>>Hi Kan,
>>>>>
>>>>> Thanks, this looks like exactly what we need. If there are no
>>>other
>>>>>comments, please send me this patch w/ a Signed-off-by and we
>can
>>>get
>>>>>it
>>>>>in tree. BTW, glad to hear you're working on the FPSWA issue
>and
>>>are
>>>>>making good progress! Thanks,
>>>>>
>>>>> Alex
>>>>
>>>>Seems OK. One small comment is that we may also remove
>>>>FORCE_CRASH completely since the assumption to add that
>>>>check doesn't exist now. Actually VHPT_CCHAIN_LOOKUP
>>>>already makes check upon VMM area to decide whether jumping
>>>>to alt_itlb_miss handler. In this case, simply removing
>>>>FORCE_CRASH line can also work. :-)
>>>
>>>If alt_itlb_fault occurred, we need ifa checking and FORCE_CRASH,
>>>don't we?
>>>Therefore I don't need to change my patch, do I?
>>>
>>
>>The check is already made before jumping to alt_itlb_miss.
>>Also architecturally there's no limitation to prevent uncacheable
>>instruction falling into that category. So I think there's no need
>>for existence of FORCE_CRASH there, right? :-)
>>
>>Thanks,
>>Kevin
>>
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|