Re: [PATCH 02/14] drm/i915/gvt: simplify vgpu configuration management

Jason Gunthorpe
2022-07-04T14:15:42Z

On Mon, Jul 04, 2022 at 02:51:32PM +0200, Christoph Hellwig wrote:

> +/*
> + * vGPU type name is defined as GVTg_Vx_y which contains the physical GPU
> + * generation type (e.g V4 as BDW server, V5 as SKL server).
> + *
> + * Depening on the physical SKU resource, we might see vGPU types like
> + * GVTg_V4_8, GVTg_V4_4, GVTg_V4_2, etc. We can create different types of
> + * vGPU on same physical GPU depending on available resource. Each vGPU
> + * type will have a different number of avail_instance to indicate how
> + * many vGPU instance can be created for this type.
> + */
> #define VGPU_MAX_WEIGHT 16
> #define VGPU_WEIGHT(vgpu_num) \
> (VGPU_MAX_WEIGHT / (vgpu_num))
>
> -static const struct {
> - unsigned int low_mm;
> - unsigned int high_mm;
> - unsigned int fence;
> -
> - /* A vGPU with a weight of 8 will get twice as much GPU as a vGPU
> - * with a weight of 4 on a contended host, different vGPU type has
> - * different weight set. Legal weights range from 1 to 16.
> - */
> - unsigned int weight;
> - enum intel_vgpu_edid edid;
> - const char *name;
> -} vgpu_types[] = {
> -/* Fixed vGPU type table */
> +static const struct intel_vgpu_config intel_vgpu_configs[] = {
> { MB_TO_BYTES(64), MB_TO_BYTES(384), 4, VGPU_WEIGHT(8), GVT_EDID_1024_768, "8" },
> { MB_TO_BYTES(128), MB_TO_BYTES(512), 4, VGPU_WEIGHT(4), GVT_EDID_1920_1200, "4" },
> { MB_TO_BYTES(256), MB_TO_BYTES(1024), 4, VGPU_WEIGHT(2), GVT_EDID_1920_1200, "2" },
> @@ -106,63 +103,34 @@ static const struct {
> */

[..]

> for (i = 0; i < num_types; ++i) {
> - if (low_avail / vgpu_types[i].low_mm == 0)
> - break;
> -
> - gvt->types[i].low_gm_size = vgpu_types[i].low_mm;
> - gvt->types[i].high_gm_size = vgpu_types[i].high_mm;
> - gvt->types[i].fence = vgpu_types[i].fence;
> + const struct intel_vgpu_config *conf = &intel_vgpu_configs[i];
>
> - if (vgpu_types[i].weight < 1 ||
> - vgpu_types[i].weight > VGPU_MAX_WEIGHT)
> + if (low_avail / conf->low_mm == 0)
> + break;
> + if (conf->weight < 1 || conf->weight > VGPU_MAX_WEIGHT)
> goto out_free_types;

This is now clearly impossible right? Maybe a BUILD_BUG_ON is all that
is needed:

#define VGPU_WEIGHT(vgpu_num) \
(VGPU_MAX_WEIGHT + BUILD_BUG_ON_ZERO((vgpu_num) > VGPU_MAX_WEIGHT) / (vgpu_num))

> + sprintf(gvt->types[i].name, "GVTg_V%u_%s",
> + GRAPHICS_VER(gvt->gt->i915) == 8 ? 4 : 5, conf->name);
> + gvt->types->conf = conf;
> + gvt->types[i].avail_instance = min(low_avail / conf->low_mm,
> + high_avail / conf->high_mm);

snprintf and check for failure?

Regardless, makes sense to me:

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason