Christoph, Keir, Jan I have a draft look at c/s 25919. It moves some mce_intel.c logic to mce.c (and remove old mce.c logic). By draft reviewing the patch I think it need more work to do, and currently it in fact would hung at AMD platform (I have no AMD platform to test), i.e, MACHINE_CHECK_SOFTIRQ --> mce_delayed_action() --> mce_action() --> ASSERT(handler_num); For AMD mce it may need add (if any misunderstanding please point to me) 1). add default handler which used at softirq context 2). add AMD vmce inject logic 3). more test Thoughts? Thanks, Jinsong
On 09/21/12 08:44, Liu, Jinsong wrote:> Christoph, Keir, Jan > > I have a draft look at c/s 25919. It moves some mce_intel.c logic to mce.c (and remove old mce.c logic). By draft reviewing the patch I think it need more work to do, and currently it in fact would hung at AMD platform (I have no AMD platform to test), i.e, MACHINE_CHECK_SOFTIRQ --> mce_delayed_action() --> mce_action() --> ASSERT(handler_num); > > For AMD mce it may need add (if any misunderstanding please point to me) > 1). add default handler which used at softirq contextThis is mcee_softirq().> 2). add AMD vmce inject logicYes, patch is sent. See http://lists.xen.org/archives/html/xen-devel/2012-09/msg01413.html> 3). more testThere are more patches in my queue. Christoph> > Thoughts? > > Thanks, > Jinsong-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632
Christoph Egger wrote:> On 09/21/12 08:44, Liu, Jinsong wrote: > >> Christoph, Keir, Jan >> >> I have a draft look at c/s 25919. It moves some mce_intel.c logic to >> mce.c (and remove old mce.c logic). By draft reviewing the patch I >> think it need more work to do, and currently it in fact would hung >> at AMD platform (I have no AMD platform to test), i.e, >> MACHINE_CHECK_SOFTIRQ --> mce_delayed_action() --> mce_action() --> >> ASSERT(handler_num); >> >> For AMD mce it may need add (if any misunderstanding please point to >> me) 1). add default handler which used at softirq context > > > This is mcee_softirq(). > >> 2). add AMD vmce inject logic > > > Yes, patch is sent. > See http://lists.xen.org/archives/html/xen-devel/2012-09/msg01413.html > >> 3). more test > > > There are more patches in my queue. > > Christoph >So, Christoph, considering you have a patches queue and would change more mce/vmce, how about wait a moment and then based your patches on my vmce patches? My vmce patches have been posted for quite some time, and have already been reviewed-rebased-tested several rounds. I think they are basically stable now, currently only need tools maintainers to have review patch4/5 (live migration tools part). ... I know I have no right asking you to do so, but we need cooperate for mce/vmce, and if you agree I would be very much appreciated. As for your mce/vmce patches, I''d like to help test at Intel''s platform to make sure it works fine. Thanks, Jinsong
On 09/24/12 12:24, Liu, Jinsong wrote:> Christoph Egger wrote: >> On 09/21/12 08:44, Liu, Jinsong wrote: >> >>> Christoph, Keir, Jan >>> >>> I have a draft look at c/s 25919. It moves some mce_intel.c logic to >>> mce.c (and remove old mce.c logic). By draft reviewing the patch I >>> think it need more work to do, and currently it in fact would hung >>> at AMD platform (I have no AMD platform to test), i.e, >>> MACHINE_CHECK_SOFTIRQ --> mce_delayed_action() --> mce_action() --> >>> ASSERT(handler_num); >>> >>> For AMD mce it may need add (if any misunderstanding please point to >>> me) 1). add default handler which used at softirq context >> >> >> This is mcee_softirq(). >> >>> 2). add AMD vmce inject logic >> >> >> Yes, patch is sent. >> See http://lists.xen.org/archives/html/xen-devel/2012-09/msg01413.html >> >>> 3). more test >> >> >> There are more patches in my queue. >> >> Christoph >> > > So, Christoph, considering you have a patches queue and would> change more mce/vmce, how about wait a moment and then based your > patches on my vmce patches? My vmce patches have been posted for > quite some time, and have already been reviewed-rebased-tested > several rounds. I think they are basically stable now, currently only > need tools maintainers to have review patch4/5 (live migration tools > part).> > ... I know I have no right asking you to do so, but we need cooperate> for mce/vmce, and if you agree I would be very much appreciated. As > for your mce/vmce patches, I''d like to help test at Intel''s platform > to make sure it works fine.I don''t mind in which order the patches go upstream - yours first, mine first or somehow interleaved.. It is just important to me that you take my comments (to patch 2/5) into account to minimize the required rebase-retest effort. Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632
Christoph Egger wrote:> On 09/24/12 12:24, Liu, Jinsong wrote: > >> Christoph Egger wrote: >>> On 09/21/12 08:44, Liu, Jinsong wrote: >>> >>>> Christoph, Keir, Jan >>>> >>>> I have a draft look at c/s 25919. It moves some mce_intel.c logic >>>> to mce.c (and remove old mce.c logic). By draft reviewing the >>>> patch I think it need more work to do, and currently it in fact >>>> would hung at AMD platform (I have no AMD platform to test), i.e, >>>> MACHINE_CHECK_SOFTIRQ --> mce_delayed_action() --> mce_action() >>>> --> ASSERT(handler_num); >>>> >>>> For AMD mce it may need add (if any misunderstanding please point >>>> to me) 1). add default handler which used at softirq context >>> >>> >>> This is mcee_softirq(). >>> >>>> 2). add AMD vmce inject logic >>> >>> >>> Yes, patch is sent. >>> See >>> http://lists.xen.org/archives/html/xen-devel/2012-09/msg01413.html >>> >>>> 3). more test >>> >>> >>> There are more patches in my queue. >>> >>> Christoph >>> >> >> So, Christoph, considering you have a patches queue and would > >> change more mce/vmce, how about wait a moment and then based your >> patches on my vmce patches? My vmce patches have been posted for >> quite some time, and have already been reviewed-rebased-tested >> several rounds. I think they are basically stable now, currently only >> need tools maintainers to have review patch4/5 (live migration tools >> part). > >> >> ... I know I have no right asking you to do so, but we need cooperate > >> for mce/vmce, and if you agree I would be very much appreciated. As >> for your mce/vmce patches, I''d like to help test at Intel''s platform >> to make sure it works fine. > > I don''t mind in which order the patches go upstream - > yours first, mine first or somehow interleaved.. > > It is just important to me that you take my comments (to patch 2/5) > into account to minimize the required rebase-retest effort. > > ChristophThanks Christoph! I have add a patch injecting vmce for AMD per your comments, will sent out later. Best Regards, Jinsong