From: "Liu, Yong" <yong.liu@intel.com>
To: Herbert Guan <herbert.guan@arm.com>, "dts@dpdk.org" <dts@dpdk.org>
Cc: "phil.yang@arm.com" <phil.yang@arm.com>,
"jianbo.liu@arm.com" <jianbo.liu@arm.com>
Subject: Re: [dts] [PATCH] framework: enable AArch64 and add more options for libvirt
Date: Fri, 22 Dec 2017 08:09:46 +0000 [thread overview]
Message-ID: <86228AFD5BCD8E4EBFD2B90117B5E81E62F3804E@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <1512452777-3982-1-git-send-email-herbert.guan@arm.com>
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@dpdk.org] On Behalf Of Herbert Guan
> Sent: Tuesday, December 05, 2017 1:46 PM
> To: dts@dpdk.org
> Cc: phil.yang@arm.com; jianbo.liu@arm.com; Herbert Guan
> <herbert.guan@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@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
next prev parent reply other threads:[~2017-12-22 8:09 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-05 5:46 Herbert Guan
2017-12-22 8:09 ` Liu, Yong [this message]
2017-12-22 9:53 ` Herbert Guan
2018-01-02 2:21 ` Liu, Yong
2018-01-03 4:40 ` [dts] [PATCH v2] " Herbert Guan
2018-01-03 16:44 ` Liu, Yong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86228AFD5BCD8E4EBFD2B90117B5E81E62F3804E@SHSMSX103.ccr.corp.intel.com \
--to=yong.liu@intel.com \
--cc=dts@dpdk.org \
--cc=herbert.guan@arm.com \
--cc=jianbo.liu@arm.com \
--cc=phil.yang@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).