Re: [PATCH igt] Add the new case for create GVT-g Linux guest based on KVMGT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jan 01, 2017 at 12:33:53PM +0000, Xu, Terrence wrote:
> > On Sat, Dec 31, 2016 at 01:24:53AM +0800, Terrence Xu wrote:
> > > GVT-g (Intel® Graphics Virtualization Technology) is a full GPU
> > > virtualization solution with mediated pass-through support.
> > >
> > > This case is for create basic Linux guest based on KVMGT with VFIO
> > > framework, it including create vgpu, create guest, check IP address,
> > > destroy guest, remove vgpu,check dmesg log, etc functions.
> > >
> > > Signed-off-by: Terrence Xu <terrence.xu@xxxxxxxxx>
> > > Signed-off-by: Benyu Xu <benyux.xu@xxxxxxxxx>
> > > ---
> > >  tests/Makefile.sources |   1 +
> > >  tests/gvtg_guest.c     | 323
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 324 insertions(+)
> > >  create mode 100644 tests/gvtg_guest.c
> > >
> > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources index
> > > 04dd2d5..09b936e 100644
> > > --- a/tests/Makefile.sources
> > > +++ b/tests/Makefile.sources
> > > @@ -219,6 +219,7 @@ TESTS_progs = \
> > >  	prime_udl \
> > >  	drv_module_reload \
> > >  	kms_sysfs_edid_timing \
> > > +    gvtg_guest \
> > >  	$(NULL)
> > >
> > >  # IMPORTANT: The ZZ_ tests need to be run last!
> > > diff --git a/tests/gvtg_guest.c b/tests/gvtg_guest.c new file mode
> > > 100644 index 0000000..b9e6304
> > > --- /dev/null
> > > +++ b/tests/gvtg_guest.c
> > > @@ -0,0 +1,323 @@
> > > +/*
> > > + * Copyright 2016 Intel Corporation
> > > + *   Terrence Xu <terrence.xu@xxxxxxxxx>
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person
> > > +obtaining a
> > > + * copy of this software and associated documentation files (the
> > > +"Software"),
> > > + * to deal in the Software without restriction, including without
> > > +limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute,
> > > +sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to whom
> > > +the
> > > + * Software is furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice shall be
> > > +included in
> > > + * all copies or substantial portions of the Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > +EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > +MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> > EVENT
> > > +SHALL THE
> > > + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > DAMAGES OR
> > > +OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > > +ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > > +OTHER DEALINGS
> > > + * IN THE SOFTWARE.
> > > + */
> > > +
> > > +/*
> > > + * This program is intended for testing of validate GVT-g virtual
> > > +machine
> > > + * creation of allow for testing of KVMGT / XenGT.
> > > + *
> > > + * TODO:
> > > + * Enable more GVT-g related test cases.
> > > + */
> > > +#ifdef HAVE_CONFIG_H
> > > +#include "config.h"
> > > +#endif
> > > +
> > > +#include "igt.h"
> > > +#include <errno.h>
> > > +#include <getopt.h>
> > > +#include <math.h>
> > > +#include <stdint.h>
> > > +#include <stdbool.h>
> > > +#include <strings.h>
> > > +#include <unistd.h>
> > > +#include <termios.h>
> > > +#include <sys/poll.h>
> > > +#include <sys/time.h>
> > > +#include <sys/ioctl.h>
> > > +#include <sys/types.h>
> > > +#include <sys/stat.h>
> > > +#include <string.h>
> > > +#include <stdlib.h>
> > > +#include <signal.h>
> > > +#include <stdio.h>
> > > +#include <stdlib.h>
> > > +#include <time.h>
> > > +#include <unistd.h>
> > > +#include <pwd.h>
> > > +#include <limits.h>
> > > +
> > > +#define RANDOM(x) (rand() % x)
> > > +
> > > +static unsigned char mac_addr[17] = {'0', '0', ':', '0', '0', ':',
> > > +    '0', '0', ':', '0', '0', ':', '0', '0', ':', '0', '0'}; static
> > > +char uuid[40]; static char guest_ip[32];
> > > +
> > > +static char qemu_path[PATH_MAX] = {0}; static char hda_path[PATH_MAX]
> > > += {0}; static char bios_path[PATH_MAX] = {0};
> > > +
> > > +static int super_system(const char *cmd, char *retmsg, int msg_len) {
> > > +    FILE *fp;
> > > +    int res = -1;
> > > +    if (cmd == NULL || retmsg == NULL || msg_len < 0) {
> > > +        igt_info("Error: %s system paramer invalid!\n", __func__);
> > > +        return 1;
> > > +    }
> > > +    fp = popen(cmd, "r");
> > > +    if (fp == NULL) {
> > > +        perror("popen");
> > > +        igt_info("Error: %s popen error: %s\n", __func__, strerror(errno));
> > > +        return 2;
> > > +    } else {
> > > +        memset(retmsg, 0, msg_len);
> > > +        while (fgets(retmsg, msg_len, fp)) {
> > > +            ;
> > > +        };
> > > +        res = pclose(fp);
> > > +        if (res == -1) {
> > > +            igt_info("Error:%s close popen file pointer fp error!\n", __func__);
> > > +            return 3;
> > > +        }
> > > +        retmsg[strlen(retmsg) - 1] = '\0';
> > > +        return 0;
> > > +    }
> > > +}
> > > +
> > > +static void create_guest(void)
> > > +{
> > > +    char create_qcow_cmd[PATH_MAX] = {0};
> > > +    char create_vgpu_cmd[PATH_MAX] = {0};
> > > +    char create_instance_cmd[PATH_MAX] = {0};
> > > +
> > > +    sprintf(create_qcow_cmd, "qemu-img create -b %s -f qcow2 %s.qcow2",
> > > +            hda_path, hda_path);
> > > +    sprintf(create_vgpu_cmd, "echo \"%s\" >
> > /sys/bus/pci/devices/0000:00:02.0/"
> > > +           "mdev_supported_types/i915-GVTg_V4_1/create", uuid);
> > > +    sprintf(create_instance_cmd, "%s -m 2048 -smp 2 -M pc -name
> > gvtg_guest"
> > > +           " -hda %s.qcow2 -bios %s -enable-kvm --net nic,macaddr=%s -net"
> > > +           " tap,script=/etc/qemu-ifup -vga none -device isa-vga -k en-us"
> > > +           " -serial stdio -vnc :1 -machine kernel_irqchip=on -global"
> > > +           " PIIX4_PM.disable_s3=1 -global PIIX4_PM.disable_s4=1 -cpu host"
> > > +           " -usb -usbdevice tablet -device vfio-pci,sysfsdev="
> > > +           "/sys/bus/pci/devices/0000:00:02.0/%s &",
> > > +           qemu_path, hda_path, bios_path, mac_addr, uuid);
> > > +    system(create_qcow_cmd);
> > > +    system(create_vgpu_cmd);
> > > +    system(create_instance_cmd);
> > > +}
> > > +
> > > +static void destroy_all_guest(void)
> > > +{
> > > +    system("pkill qemu");
> > > +}
> > > +
> > > +static void remove_vgpu(void)
> > > +{
> > > +    char cmd[128] = {0};
> > > +    sprintf(cmd, "echo 1 > /sys/bus/pci/devices/0000:00:02.0/%s/remove",
> > uuid);
> > > +    system(cmd);
> > > +}
> > > +
> > > +static void gen_mac_addr(void)
> > > +{
> > > +    unsigned char mac_enum[16] = {'0', '1', '2', '3', '4', '5', '6',
> > > +        '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F'};
> > > +    unsigned short i, n = 2;
> > > +    srand(getpid());
> > > +    for (i = 2, n = 2; i < 15; i++, n++) {
> > > +        if (mac_addr[n] != ':') {
> > > +            mac_addr[n] = mac_enum[RANDOM(16)];
> > > +        }
> > > +    }
> > > +}
> > > +
> > > +static void gen_uuid(void)
> > > +{
> > > +    super_system("uuidgen", uuid, sizeof(uuid)); }
> > > +
> > > +static char *fetch_ip_by_mac(unsigned char *mac) {
> > > +    char cmd[1024] = {0};
> > > +    sprintf(cmd, "arp-scan -l -I $(ip addr show|grep inet|grep global|"
> > > +        "awk '{print $NF;}')|grep -i %s|awk '{print $1}'", mac);
> > > +    super_system(cmd, guest_ip, sizeof(guest_ip));
> > > +    return guest_ip;
> > > +}
> > > +
> > > +static int check_guest_ip(void)
> > > +{
> > > +    int timeout = 0;
> > > +    while (timeout <= 12) {
> > > +        igt_info("Try to connnect guest,the %d time.\n", timeout);
> > > +        if (timeout == 12) {
> > > +            igt_info("Cannot connect to guest.\n");
> > > +            return 1;
> > > +            break;
> > > +        }
> > > +
> > > +        fetch_ip_by_mac(mac_addr);
> > > +
> > > +        if (guest_ip[0] != '\0') {
> > > +            igt_info("Fetched guest ip address: %s.\n", guest_ip);
> > > +            break;
> > > +        }
> > > +        timeout++;
> > > +        sleep(5);
> > > +    }
> > > +    return 0;
> > > +}
> > > +
> > > +static void clear_dmesg(void)
> > > +{
> > > +    system("dmesg -c");
> > > +}
> > > +
> > > +static int check_dmesg(void)
> > > +{
> > > +    char errorlog[PATH_MAX] = {0};
> > > +
> > > +    super_system("dmesg|grep -E \"GPU HANG|gfx reset|BUG\"", errorlog,
> > > +            sizeof(errorlog));
> > > +
> > > +    if (errorlog[0] == '\0') {
> > > +        return 0;
> > > +    } else {
> > > +        return 1;
> > > +    }
> > > +}
> > > +
> > > +static const char optstr[] = "h";
> > > +
> > > +static void print_help(void)
> > > +{
> > > +    igt_info("\n [options]\n"
> > > +          "-h, --help     display usage\n"
> > > +          "-q, --qemu     the qemu path\n"
> > > +          "-a, --hda      the hda raw image / qcow path\n"
> > > +          "-b, --bios     the seabios path\n\n"
> > > +          );
> > > +}
> > > +
> > > +static void arg_mismatch(void)
> > > +{
> > > +    igt_info(" argument mismatch!\n"); }
> > > +
> > > +int main(int argc, char **argv)
> > 
> > That's not an igt testcase, and if you'd run with the testrunner it then it'd go
> > boom. Again I'm not really sure why this needs to be an igt testcase, since the
> > entire setup is quite involved, and it needs very specific circumstances to run
> > properly.
> > 
> > I guess merging this to igt makes some sense, but probably better as a free-
> > standing too or something similar.
> > -Daniel
> 
> [Xu, Terrence] Hi Daniel, since the GVT-g is upstreamed and the GVT-g
> feature testing will be merged to i915 CI system, so we must merged this
> to IGT ASAP, and it is also OK to provide the setup guide to IGT (Only
> need to provide Qemu, Seabios, guest image) without any extra change for
> Linux Kernel.
> 
> It is pretty good to merge it to igt as a test case directly, but if as
> a free-standing tool is also OK, any suggestion for if merge it as a
> tool? Create a new folder?

igt isn't our intel-only playground anymore, which means if you need to
provide setup instructions to make your testcase work, then it's not
acceptable.

It needs to at least use igt infrastructure like igt_main, it needs to
correctly detect&skip when it's not running on gvt-g capable hardware and
kernels, and it needs to auto-detect the setup stuff and again correctly
skip when that's not there.

In the end your test must be able to run on e.g. amd machines, without
crashing or failing.

> > > +{
> > > +    int opt = -1;
> > > +    const char *optstring = "hq:a:b:";
> > > +    static struct option long_options[] = {
> > > +        {"help", 0, NULL, 'h'},
> > > +        {"qemu", 1, NULL, 'q'},
> > > +        {"hda",  1, NULL, 'a'},
> > > +        {"bios", 1, NULL, 'b'},
> > > +        {0, 0, 0, 0}
> > > +    };

But atm your test even reinvents option parsing, for which we also have
helpers. So needs some serious work before it can be merged.

I still personally disagree that gvt testing needs to be done in our
overall CI: Patchwork supports multiple different CI, so there's no issue
with integrating this on your side. And it's also not really the blocker
for merging this testcase/tool.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux