[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [Xen-ia64-devel] alt_itlb_miss?



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.