public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: Borislav Petkov <bp@amd64.org>
Cc: mingo@elte.hu, tglx@linutronix.de, andreas.herrmann3@amd.com,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] x86, cacheinfo: Fix disabling of L3 cache indexes
Date: Thu, 21 Jan 2010 10:21:29 -0800	[thread overview]
Message-ID: <4B589B29.4040201@zytor.com> (raw)
In-Reply-To: <20100121163048.GB10321@aftab>

On 01/21/2010 08:30 AM, Borislav Petkov wrote:
> 
> The preexisting code using wbinvd is a bug since wbinvd has to happen on
> a core which contains the L3 cache whose indices(!) we disable.
> 

OK, please make a note in the code as to why it is (if it is) correct
not to use the PV version here... I'm not sure the above explains why it
should invoke native_wbinvd() and not wbinvd() -- the fact that is has
to be done on the right CPU is another matter, of course.

> But yeah, a wbinvd smp version is a bit problematic since
> smp_call_function_* expects a function pointer which has a void *
> argument but native_wbinvd() has no args. By the way, there's a similar
> thing in <drivers/char/agp/intel-agp.c::do_wbinvd()>

Saw that, thank you for handling that.

>> Second, it's pretty obvious that the only reason for this function at
>> all is to provide a wrapper that can be passed to smp_call_function*().
>>  It would be a lot cleaner to have a small function wbinvd_on_cpu(cpu)
>> as a wrapper for the higher-order functionality.
> 
> Done, see my next patch series.
> 

I wrote some comments on that before I saw this reply.  I think it has
some code cleanliness issues (noted in comments), but otherwise it's a
lot better.

	-hpa



  reply	other threads:[~2010-01-21 18:22 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-19 11:07 [PATCH 0/3] x86, cacheinfo, amd: L3 Cache Index Disable fixes Borislav Petkov
2010-01-19 11:07 ` [PATCH 1/3] x86, cacheinfo: Fix disabling of L3 cache indexes Borislav Petkov
2010-01-20 23:04   ` H. Peter Anvin
2010-01-21 16:30     ` Borislav Petkov
2010-01-21 18:21       ` H. Peter Anvin [this message]
2010-01-19 11:07 ` [PATCH 2/3] x86, cacheinfo: Add cache index disable sysfs attrs only to L3 caches Borislav Petkov
2010-01-19 11:07 ` [PATCH 3/3] x86, cacheinfo: Calculate L3 indexes Borislav Petkov
2010-01-20 23:15   ` H. Peter Anvin
2010-01-21 16:31     ` Borislav Petkov

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=4B589B29.4040201@zytor.com \
    --to=hpa@zytor.com \
    --cc=andreas.herrmann3@amd.com \
    --cc=bp@amd64.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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