From mboxrd@z Thu Jan  1 00:00:00 1970
From: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
Subject: Re: [PATCH V2] x86,
 amd_ucode: Verify max allowed patch size before apply
Date: Tue, 29 Apr 2014 18:56:38 -0500
Message-ID: <53603C36.8060708@amd.com>
References: <1398702918-12685-1-git-send-email-aravind.gopalakrishnan@amd.com>
	<535F78A6020000780000D340@nat28.tlf.novell.com>
	<53601AB9.8000907@amd.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <53601AB9.8000907@amd.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>, andrew.cooper3@citrix.com, xen-devel@lists.xen.org, keir@xen.org, Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Thomas.Lendacky@amd.com, Suravee.Suthikulpanit@amd.com
List-Id: xen-devel@lists.xenproject.org

On 4/29/2014 4:33 PM, Aravind Gopalakrishnan wrote:
> On 4/29/2014 3:02 AM, Jan Beulich wrote:
>
>>> @@ -123,8 +151,17 @@ static bool_t microcode_fits(const struct 
>>> microcode_amd *mc_amd, int cpu)
>>>       if ( (mc_header->processor_rev_id) != equiv_cpu_id )
>>>           return 0;
>>>   +    if ( !verify_patch_size(mc_amd->mpb_size) )
>>> +    {
>>> +        printk(XENLOG_DEBUG "microcode: patch size mismatch\n");
>>> +        return -E2BIG;
>>> +    }
>>> +
>>>       if ( mc_header->patch_id <= uci->cpu_sig.rev )
>>> -        return 0;
>>> +    {
>>> +        printk(XENLOG_DEBUG "microcode: patch is already at 
>>> required level or greater.\n");
>>> +        return -EEXIST;
>>> +    }
>>>         printk(KERN_DEBUG "microcode: CPU%d found a matching 
>>> microcode "
>>>              "update with version %#x (current=%#x)\n",
>> Honestly I'm rather hesitant to accept further generally useless
>> messages, no matter that they get printed at KERN_DEBUG only. I'd
>> much rather see these as well as the existing ones to be converted
>> to pr_debug(), thus easily enabled if someone really needs to do
>> debugging here. That's mainly because I (and I suppose other
>> developers do so to) try to run with loglvl=all wherever possible,
>> yet already on the 2x4-core box (not to speak of the newer 2x12-
>> core one) I find these rather annoying.
>
> Hmm, Okay. I'll work on this and send an updated version..
>
>

couple of ideas about implementing this:

1. something similar to mwait-idle.c:
+#ifdef DEBUG
+# define pr_debug(fmt...) printk(KERN_DEBUG fmt)
+#else
+# define pr_debug(fmt...)
+#endif

2. a custom_param: something like mce_verbosity:
(reference code)
+//arav
+int ucode_verbosity;
+static void __init ucode_set_verbosity(char *str)
+{
+    if (strcmp("verbose", str) == 0)
+        ucode_verbosity = 1;
+    else
+        printk(KERN_DEBUG "Microcode verbosity level %s not recognised"
+               "use ucode_verbosity=verbose", str);
+}
+custom_param("ucode_verbosity", ucode_set_verbosity);
+
+#define pr_debug(v, fmt...) do {       \
+        if ((v) <= ucode_verbosity) \
+            printk(fmt);       \
+        } while (0)
+

and using: (example)
+        pr_debug(1,"microcode: patch size mismatch\n");

We can probably put it in a 'microcode.h' (?) file and extend functionality
to microcode_intel as well.. (microcode_intel will only need some very 
minor edits)

I have tried both options and they work fine.
(well, second option i know works for sure on AMD :) )

Thoughts?

-Aravind.