On Fri, 2018-08-10 at 10:17 -0500, Eric Blake wrote:
> On 08/10/2018 09:06 AM, Robert Hoo wrote:
>
> In the subject: s/funcitons/functions/
>
> Also, it may be worth using a topic prefix (most of our commit messages
> resemble:
>
> topic: Description of patch
>
> to make it easier to spot patches by topic).
>
> > Add an util function feature_word_description(), which help construct the
> > string
>
> s/an util/a util/
> s/help/helps/
>
Thanks Eric, will fix these typos in v2.
> > describing the feature word (both CPUID and MSR types).
> >
> > report_unavailable_features(): add MSR_FEATURE_WORD type support.
> > x86_cpu_get_feature_words(): limit to CPUID_FEATURE_WORD only.
> > x86_cpu_get_supported_feature_word(): add MSR_FEATURE_WORD type support.
> > x86_cpu_adjust_feat_level(): assert the requested feature must be
> > CPUID_FEATURE_WORD type.
> >
> > Signed-off-by: Robert Hoo <robert...@linux.intel.com>
> > ---
> > target/i386/cpu.c | 77
> > +++++++++++++++++++++++++++++++++++++++++--------------
> > 1 file changed, 58 insertions(+), 19 deletions(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index f7c70d9..21ed599 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -3024,21 +3024,51 @@ static const TypeInfo host_x86_cpu_type_info = {
> >
> > #endif
> >
> > +/*
> > +*caller should have input str no less than 64 byte length.
> > +*/
> > +#define FEATURE_WORD_DESCPTION_LEN 64
>
> s/DESCPTION/DESCRIPTION/
>
> > +static int feature_word_description(char str[], FeatureWordInfo *f,
> > + uint32_t bit)
> > +{
>
> Prone to buffer overflow if the caller doesn't pass in the length.
> Would it be better to just g_strdup_printf() into a malloc'd result
> instead of trying to snprintf()'ing into a buffer that you hope the
> caller sized large enough?
>
Oh, wasn't aware of such good util functions. Sure I'd like to use them.
Is there some web page introducing them?
Eduardo/Paolo, do you have more comments?
> > + int ret;
> > +
> > + assert(f->type == CPUID_FEATURE_WORD ||
> > + f->type == MSR_FEATURE_WORD);
> > + switch (f->type) {
> > + case CPUID_FEATURE_WORD:
> > + {
> > + const char *reg = get_register_name_32(f->cpuid.reg);
> > + assert(reg);
> > + ret = snprintf(str, FEATURE_WORD_DESCPTION_LEN,
> > + "CPUID.%02XH:%s%s%s [bit %d]",
> > + f->cpuid.eax, reg,
> > + f->feat_names[bit] ? "." : "",
> > + f->feat_names[bit] ? f->feat_names[bit] : "", bit);
> > + break;
> > + }
> > + case MSR_FEATURE_WORD:
> > + ret = snprintf(str, FEATURE_WORD_DESCPTION_LEN,
> > + "MSR(%xH).%s [bit %d]",
> > + f->msr.index,
> > + f->feat_names[bit] ? f->feat_names[bit] : "", bit);
> > + break;
> > + }
>
