From: "Yinghai Lu" <yhlu.kernel@gmail.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: "Neil Horman" <nhorman@tuxdriver.com>,
"Neil Horman" <nhorman@redhat.com>,
"Ben Woodard" <woodard@redhat.com>,
kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
"Andi Kleen" <ak@suse.de>,
hbabu@us.ibm.com, "Andi Kleen" <andi@firstfloor.org>
Subject: Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Date: Tue, 11 Dec 2007 10:00:00 -0800 [thread overview]
Message-ID: <86802c440712111000l39291faaka797bc4ee22f798a@mail.gmail.com> (raw)
In-Reply-To: <m1ve75z1v3.fsf@ebiederm.dsl.xmission.com>
On Dec 11, 2007 7:29 AM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>
> Neil Horman <nhorman@tuxdriver.com> writes:
>
> > On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote:
> >> Neil Horman <nhorman@tuxdriver.com> writes:
> >>
> >> Almost there.
> > cool! :)
> >
> > <snip>
> >>
> >> We should not need check_hypertransport_config as the generic loop
> >> now does the work for us.
> >> > +
> >> > static void __init nvidia_bugs(void)
> >> > {
> >> > #ifdef CONFIG_ACPI
> >> > @@ -83,15 +127,25 @@ static void __init ati_bugs(void)
> >> > #endif
> >> > }
> >> >
> >> > +static void __init amd_host_bugs(void)
> >> > +{
> >> > + printk(KERN_CRIT "IN AMD_HOST_BUGS\n");
> >> > + check_hypertransport_config();
> >> > +}
> >>
> >> Likewise this function is unneeded and the printk is likely confusing
> >> for users.
> >>
> > Copy that. Fixed
> >
> > <snip>
> >> > {}
> >> So make that fix_hypertransport_config and we should be good.
> > Done
> >
> >>
> >> We don't need to shift device. Although we can do:
> >> device_vendor = read_pci_config(num, slot, func, PCI_VENDOR_ID);
> >> device = device_vendor >> 16;
> >> vendor = device_vendor & 0xffff;
> >>
> > I'm not so sure about this. In my testing, it was clear that I needed to do a
> > shift on device to make valid comparisons to the defined PCI_DEVICE_* macros.
> > The origional code had to do the same thing with the class field, which is
> > simmilarly positioned in the pci config space.
>
> Ok. I just looked at read_pci_config. It doesn't do the right thing for
> a non-aligned 32bit access. (Not that I am convinced there is a right
> thing we can do). Please make this read_pci_config_16 instead
> and you won't need the shift.
>
> Either that or as I earlier suggested just do a 32bit read from offset 0
> and use shifts and masks to get vendor and device fields.
>
> The current code doing a shift where none should be needed (because
> we ignore the two low order bits in our read) is totally weird
> when looking at it.
>
> > Other than that, new patch attached. Enables the detection of AMD
> > hypertransport functions and checks for the proper quirk just as before, and
> > incoporates your comments above Eric, as well as yours Yinghai.
>
> You almost got YH's comment. You need return 2 for the old functions
> so we don't try and apply a per chipset fixup for every device in
> the system.
>
> I'm actually inclined to remove the return magic and just do something
> like:
> static fix_applied;
> if (fix_applied++)
> return;
> In those functions that should be called only once.
it seems we need to have two tables. one for northbridge (sweep all
the NB_K8) and another for SB ( like Nvidia, ati..., one touch and
leave)
YH
next prev parent reply other threads:[~2007-12-11 18:00 UTC|newest]
Thread overview: 100+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-11-27 1:47 [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu Neil Horman
2007-11-27 4:12 ` Eric W. Biederman
2007-11-27 13:13 ` Neil Horman
2007-11-27 13:28 ` Eric W. Biederman
2007-11-27 13:45 ` Andi Kleen
2007-11-27 14:28 ` Neil Horman
2007-11-27 14:43 ` Andi Kleen
2007-11-27 14:48 ` Neil Horman
2007-11-27 15:24 ` Andi Kleen
2007-11-27 15:30 ` Eric W. Biederman
2007-11-27 16:45 ` Neil Horman
2007-11-27 20:50 ` Ben Woodard
2007-11-27 21:05 ` Neil Horman
2007-11-27 22:38 ` Eric W. Biederman
2007-11-27 23:15 ` Ben Woodard
2007-11-28 0:15 ` Eric W. Biederman
2007-11-27 23:40 ` Neil Horman
2007-11-28 0:43 ` Eric W. Biederman
2007-11-28 15:54 ` Neil Horman
2007-11-27 14:56 ` Eric W. Biederman
2007-11-27 15:34 ` Neil Horman
2007-11-27 18:41 ` Ben Woodard
2007-11-27 19:42 ` Neil Horman
2007-11-27 20:00 ` Vivek Goyal
2007-11-27 20:52 ` Neil Horman
2007-11-27 22:24 ` Andi Kleen
2007-11-27 23:24 ` Ben Woodard
2007-11-27 23:56 ` Andi Kleen
2007-11-28 15:36 ` Vivek Goyal
2007-11-28 16:02 ` Neil Horman
2007-11-28 17:36 ` Eric W. Biederman
2007-11-28 18:16 ` Neil Horman
2007-11-28 19:05 ` Vivek Goyal
2007-11-28 19:42 ` Eric W. Biederman
2007-11-28 21:09 ` Neil Horman
2007-11-28 23:27 ` Eric W. Biederman
2007-11-30 2:16 ` Ben Woodard
2007-11-30 2:54 ` Eric W. Biederman
2007-11-30 8:59 ` Yinghai Lu
2007-11-30 14:35 ` Vivek Goyal
2007-11-30 14:32 ` Neil Horman
2007-11-30 2:12 ` Ben Woodard
2007-11-30 14:42 ` Vivek Goyal
2007-11-30 14:51 ` Neil Horman
2007-12-06 21:39 ` Neil Horman
2007-12-06 22:11 ` Vivek Goyal
2007-12-07 0:10 ` Neil Horman
2007-12-07 14:39 ` Vivek Goyal
2007-12-07 14:53 ` Neil Horman
2007-12-07 15:16 ` Vivek Goyal
2007-12-07 15:53 ` Neil Horman
2007-12-07 18:46 ` Eric W. Biederman
2007-12-07 0:33 ` Eric W. Biederman
2007-12-07 2:04 ` Neil Horman
2007-12-07 8:50 ` Yinghai Lu
2007-12-07 9:22 ` Yinghai Lu
2007-12-07 14:21 ` Neil Horman
2007-12-07 17:58 ` Neil Horman
2007-12-07 19:19 ` yhlu
2007-12-07 20:13 ` Neil Horman
2007-12-10 15:39 ` Neil Horman
2007-12-10 16:20 ` Vivek Goyal
2007-12-11 1:17 ` Eric W. Biederman
2007-12-11 1:08 ` Eric W. Biederman
2007-12-11 3:43 ` Neil Horman
2007-12-11 4:48 ` Eric W. Biederman
2007-12-11 6:31 ` Yinghai Lu
2007-12-11 14:39 ` Neil Horman
2007-12-11 15:29 ` Eric W. Biederman
2007-12-11 18:00 ` Yinghai Lu [this message]
2007-12-11 18:29 ` Neil Horman
2007-12-11 18:45 ` Yinghai Lu
2007-12-11 18:22 ` Neil Horman
2007-12-11 18:46 ` Eric W. Biederman
2007-12-11 19:24 ` Neil Horman
2007-12-11 19:51 ` Yinghai Lu
2007-12-11 20:59 ` Neil Horman
2007-12-12 0:16 ` Ben Woodard
2007-12-12 0:52 ` Neil Horman
2007-12-12 1:07 ` Yinghai Lu
2007-12-12 8:43 ` [PATCH] k8: Enable legacy irqs with extended cpu ids Eric W. Biederman
2007-12-12 14:21 ` [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu Andi Kleen
2007-12-12 15:55 ` Neil Horman
2007-12-12 16:07 ` Andi Kleen
2007-12-12 19:43 ` Eric W. Biederman
2007-12-12 20:22 ` Neil Horman
2007-12-12 21:32 ` Eric W. Biederman
2007-12-13 14:39 ` Neil Horman
2007-12-13 15:16 ` Andi Kleen
2007-12-13 15:32 ` Neil Horman
2007-12-17 11:38 ` Neil Horman
2007-12-18 0:13 ` Eric W. Biederman
2007-12-17 15:16 ` Ingo Molnar
2007-12-17 15:47 ` Neil Horman
2007-12-07 18:36 ` Eric W. Biederman
2007-12-07 18:48 ` Neil Horman
2007-11-27 13:53 ` Neil Horman
2007-11-27 10:55 ` Andi Kleen
2007-11-27 11:19 ` Eric W. Biederman
2007-11-27 13:28 ` Neil Horman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86802c440712111000l39291faaka797bc4ee22f798a@mail.gmail.com \
--to=yhlu.kernel@gmail.com \
--cc=ak@suse.de \
--cc=andi@firstfloor.org \
--cc=ebiederm@xmission.com \
--cc=hbabu@us.ibm.com \
--cc=kexec@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nhorman@redhat.com \
--cc=nhorman@tuxdriver.com \
--cc=woodard@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).