[dts] [PATCH] framework: enable AArch64 and add more options for libvirt

Liu, Yong yong.liu at intel.com
Fri Dec 22 09:09:46 CET 2017


Herbert, thanks for your patch.  I'm not very clear about what is must for AArch64 support and what is for enabling your environment.
Please separate them in different functions. 

Thanks,
Marvin 

> -----Original Message-----
> From: dts [mailto:dts-bounces at dpdk.org] On Behalf Of Herbert Guan
> Sent: Tuesday, December 05, 2017 1:46 PM
> To: dts at dpdk.org
> Cc: phil.yang at arm.com; jianbo.liu at arm.com; Herbert Guan
> <herbert.guan at arm.com>
> Subject: [dts] [PATCH] framework: enable AArch64 and add more options for
> libvirt
> 
> 1) Add the libvirt support on AArch64 platforms
> 2) Add 'loader' and 'nvram' option support in 'os' section
> 3) Add 'opt_bus', 'opt_dev' and 'opt_controller' option support in
>    'disk' section.  Change 'type' to 'opt_format' to get aligined
>    with option naming in qemu_kvm.
> 4) Add serial 'serial_option' option support for serial console
> 5) Add domain parsing in __parse_pci()
> 6) Function add_vm_device() supports both VF (new) and PF devices now
> 
> Signed-off-by: Herbert Guan <herbert.guan at arm.com>
> ---
>  conf/vm_power_manager.cfg |   4 +-
>  framework/qemu_libvirt.py | 152 ++++++++++++++++++++++++++++++++++++++---
> -----
>  2 files changed, 128 insertions(+), 28 deletions(-)
>  mode change 100644 => 100755 conf/vm_power_manager.cfg
>  mode change 100644 => 100755 framework/qemu_libvirt.py
> 
> diff --git a/conf/vm_power_manager.cfg b/conf/vm_power_manager.cfg
> old mode 100644
> new mode 100755
> index 9c8f87a..7d5d9b6
> --- a/conf/vm_power_manager.cfg
> +++ b/conf/vm_power_manager.cfg
> @@ -7,7 +7,7 @@
>  #   size: 4096
>  # disk
>  #   file: absolute path to disk image
> -#   type: disk image format
> +#   opt_format: [ raw | qcow2 | ... ]  #disk image format
>  # login
>  #   user: user name to login into VM
>  #   password: passwork to login into VM
> @@ -25,7 +25,7 @@ cpu =
>  mem =
>      size=4096;
>  disk =
> -    file=/storage/vm-image/vm0.img,type=raw;
> +    file=/storage/vm-image/vm0.img,opt_format=raw;
>  login =
>      user=root,password=tester;
>  [vm1]
> diff --git a/framework/qemu_libvirt.py b/framework/qemu_libvirt.py
> old mode 100644
> new mode 100755
> index ed8e0e6..18dcd8b
> --- a/framework/qemu_libvirt.py
> +++ b/framework/qemu_libvirt.py
> @@ -57,6 +57,7 @@ class LibvirtKvm(VirtBase):
> 
>          # disk and pci device default index
>          self.diskindex = 'a'
> +        self.controllerindex = 0
>          self.pciindex = 10
> 
>          # configure root element
> @@ -82,7 +83,12 @@ class LibvirtKvm(VirtBase):
> 
>          # set some default values for vm,
>          # if there is not the values of the specified options
> -        self.set_vm_default()
> +        arch = self.host_session.send_expect('uname -m', '# ')
> +        set_default_func = getattr(self, 'set_vm_default_' + arch)
> +        if callable(set_default_func):
> +            set_default_func()
> +        else:
> +            self.set_vm_default()

I'm not sure how much is the difference between x86 and AArch64 platform. Look like somethings are common for example graphic and control interface.
Could we separate arch related setting in separated function and keep those common in another function?

> 
>      def get_qemu_emulator(self):
>          """
> @@ -98,6 +104,13 @@ class LibvirtKvm(VirtBase):
>          """
>          check and setup host virtual ability
>          """
> +        arch = self.host_session.send_expect('uname -m', '# ')
> +        if arch == 'aarch64':
> +            out = self.host_session.send_expect('service libvirtd status',
> "# ")
> +            if 'active (running)' not in out:
> +                return False
> +            return True
> +

Herbert, libvirt status has been checked in function has_virtual_ability when initialize LibvirtKvm object. 
Why do this check again?

>          out = self.host_session.send_expect('cat /proc/cpuinfo | grep
> flags',
>                                              '# ')
>          rgx = re.search(' vmx ', out)
> @@ -201,6 +214,47 @@ class LibvirtKvm(VirtBase):
>                                  ',name=org.qemu.guest_agent.0'})
>          self.qga_sock_path = '/tmp/%s_qga0.sock' % self.vm_name
> 
> +    def add_vm_os(self, **options):
> +        os = self.domain.find('os')
> +        if 'loader' in options.keys():
> +            loader = ET.SubElement(
> +            os, 'loader', {'readonly': 'yes', 'type': 'pflash'})
> +            loader.text = options['loader']
> +        if 'nvram' in options.keys():
> +            nvram = ET.SubElement(os, 'nvram')
> +            nvram.text = options['nvram']
> +

Where this function is called? 

> +
> +    def set_vm_default_aarch64(self):
> +        os = ET.SubElement(self.domain, 'os')
> +        type = ET.SubElement(
> +            os, 'type', {'arch': 'aarch64', 'machine': 'virt'})
> +        type.text = 'hvm'
> +        ET.SubElement(os, 'boot', {'dev': 'hd'})
> +
> +        features = ET.SubElement(self.domain, 'features')
> +        ET.SubElement(features, 'acpi')
> +        ET.SubElement(features, 'gic', {'version': '3'})
> +
> +        ET.SubElement(self.domain, 'cpu',
> +            {'mode': 'host-passthrough', 'check': 'none'})
> +
> +        # qemu-kvm for emulator
> +        device = ET.SubElement(self.domain, 'devices')
> +        ET.SubElement(device, 'emulator').text = self.qemu_emulator
> +
> +        # graphic device
> +        #ET.SubElement(device, 'graphics', {
> +        #              'type': 'vnc', 'port': '-1', 'autoport': 'yes'})
> +        # qemu guest agent
> +        self.add_vm_qga(None)
> +
> +        # add default control interface
> +        if not self.__default_nic:
> +            def_nic = {'type': 'nic', 'opt_hostfwd': ''}
> +            self.add_vm_net(**def_nic)
> +            self.__default_nic = True
> +
>      def set_vm_default(self):
>          os = ET.SubElement(self.domain, 'os')
>          type = ET.SubElement(
> @@ -273,17 +327,44 @@ class LibvirtKvm(VirtBase):
>              return False
> 
>          ET.SubElement(disk, 'source', {'file': options['file']})
> -        if 'type' not in options:
> +        if 'opt_format' not in options:
>              disk_type = 'raw'
>          else:
> -            disk_type = options['type']
> +            disk_type = options['opt_format']
> 
>          ET.SubElement(disk, 'driver', {'name': 'qemu', 'type': disk_type})
> 
> +        if 'opt_bus' not in options:
> +            bus = 'virtio'
> +        else:
> +            bus = options['opt_bus']
> +        if 'opt_dev' not in options:
> +            dev = 'vd%c' % self.diskindex
> +            self.diskindex = chr(ord(self.diskindex) + 1)
> +        else:
> +            dev = options['opt_dev']
>          ET.SubElement(
> -            disk, 'target', {'dev': 'vd%c' % self.diskindex, 'bus':
> 'virtio'})
> +            disk, 'target', {'dev': dev, 'bus': bus})
> +
> +        if 'opt_controller' in options:
> +            controller = ET.SubElement(devices, 'controller',
> +                {'type': bus,
> +                'index': hex(self.controllerindex)[2:],
> +                'model': options['opt_controller']})
> +            self.controllerindex += 1
> +            ET.SubElement(controller, 'address',
> +                {'type': 'pci', 'domain': '0x0000', 'bus':
> hex(self.pciindex),
> +                'slot': '0x00', 'function': '0x00'})
> +            self.pciindex += 1
> 
> -        self.diskindex = chr(ord(self.diskindex) + 1)
> +    def add_vm_serial_port(self, **options):
> +        if 'enable' in options.keys():
> +            if options['enable'].lower() == 'yes':
> +                devices = self.domain.find('devices')
> +                serial = ET.SubElement(devices, 'serial', {'type': 'pty'})
> +                ET.SubElement(serial, 'target', {'port': '0'})
> +                console = ET.SubElement(devices, 'console', {'type':
> 'pty'})
> +                ET.SubElement(console, 'target', {'type': 'serial',
> 'port': '0'})
> 

In qemu kvm module, we are using unix domain socket for serial device. Can you align to this?
Please let device type should be configurable, we plan to add more types later.

>      def add_vm_login(self, **options):
>          """
> @@ -305,14 +386,23 @@ class LibvirtKvm(VirtBase):
>      def __parse_pci(self, pci_address):
>          pci_regex = r"([0-9a-fA-F]{1,2}):([0-9a-fA-F]{1,2})" + \
>              ".([0-9a-fA-F]{1,2})"
> +        pci_regex_domain = r"([0-9a-fA-F]{1,4}):([0-9a-fA-F]{1,2}):" + \
> +            "([0-9a-fA-F]{1,2}).([0-9a-fA-F]{1,2})"
>          m = re.match(pci_regex, pci_address)
> -        if m is None:
> -            return None
> -        bus = m.group(1)
> -        slot = m.group(2)
> -        func = m.group(3)
> -
> -        return (bus, slot, func)
> +        if m is not None:
> +            bus = m.group(1)
> +            slot = m.group(2)
> +            func = m.group(3)
> +            dom  = '0'
> +            return (bus, slot, func, dom)
> +        m = re.match(pci_regex_domain, pci_address)
> +        if m is not None:
> +            bus = m.group(2)
> +            slot = m.group(3)
> +            func = m.group(4)
> +            dom  = m.group(1)
> +            return (bus, slot, func, dom)
> +        return None
> 
>      def add_vm_device(self, **options):
>          """
> @@ -325,35 +415,45 @@ class LibvirtKvm(VirtBase):
>                                     'mode': 'subsystem', 'type': 'pci',
>                                     'managed': 'yes'})
> 
> -        if 'pf_idx' not in options.keys():
> -            print utils.RED("Missing device index for device option!!!")
> -            return False
> -
> -        pf = int(options['pf_idx'])
> -        if pf > len(self.host_dut.ports_info):
> -            print utils.RED("PF device index over size!!!")
> +        if 'pf_idx' in options.keys():
> +            pf = int(options['pf_idx'])
> +            if pf > len(self.host_dut.ports_info):
> +                print utils.RED("PF device index over size!!!")
> +                return False

Previous check sentence is incorrect:) Should be "if pf >= len(self.host_dut.ports_info):". 
Most of suites are just using option "opt_host" to specify the pci address of device. So I'm planning to remove pf_idx option, what's your idea for that?

> +            pci_addr = self.host_dut.ports_info[pf]['pci']
> +        elif 'vf_idx' in options.keys():
> +            vf = int(options['vf_idx'])
> +            if vf > len(self.host_dut.ports_info):
> +                print utils.RED("VF device index over size!!!")
> +                return False
> +            if 'vf_id' in options.keys():
> +                vf_id = int(options['vf_id'])
> +            else:
> +                vf_id = 0
> +            pci_addr =
> self.host_dut.ports_info[vf]['sriov_vfs_pci'][vf_id]
> +        else:
> +            print utils.RED("Missing pf/vf device index for device
> option!!!")
>              return False
> 
> -        pci_addr = self.host_dut.ports_info[pf]['pci']
> 
>          pci = self.__parse_pci(pci_addr)
>          if pci is None:
>              return False
> -        bus, slot, func = pci
> +        bus, slot, func, dom = pci
> 
>          source = ET.SubElement(hostdevice, 'source')
>          ET.SubElement(source, 'address', {
> -                      'domain': '0x0', 'bus': '0x%s' % bus,
> +                      'domain': '0x%s' % dom, 'bus': '0x%s' % bus,
>                        'slot': '0x%s' % slot,
>                        'function': '0x%s' % func})
>          if 'guestpci' in options.keys():
>              pci = self.__parse_pci(options['guestpci'])
>              if pci is None:
>                  return False
> -            bus, slot, func = pci
> +            bus, slot, func, dom = pci
>              ET.SubElement(hostdevice, 'address', {
> -                          'type': 'pci', 'domain': '0x0', 'bus': '0x%s' %
> bus,
> -                          'slot': '0x%s' % slot, 'function': '0x%s' %
> func})
> +                  'type': 'pci', 'domain': '0x%s' % dom, 'bus': '0x%s' %
> bus,
> +                  'slot': '0x%s' % slot, 'function': '0x%s' % func})
>              # save host and guest pci address mapping
>              pci_map = {}
>              pci_map['hostpci'] = pci_addr
> @@ -397,7 +497,7 @@ class LibvirtKvm(VirtBase):
>              pci = self.__parse_pci(options['opt_addr'])
>              if pci is None:
>                  return False
> -            bus, slot, func = pci
> +            bus, slot, func, dom = pci
>              ET.SubElement(qemu, 'qemu:arg',
>                            {'value': 'nic,model=e1000,addr=0x%s' % slot})
>          else:
> --
> 1.8.3.1



More information about the dts mailing list