Date   

Re: [PATCH v4 7/8] hv: vrtc calibrate function

Junjie Mao
 

"Zhao, Yuanyuan" <yuanyuan.zhao@...> writes:

On 5/6/2022 10:23 AM, Junjie Mao wrote:
"Zhao, Yuanyuan" <yuanyuan.zhao@...> writes:

On 5/6/2022 9:46 AM, Junjie Mao wrote:
Yuanyuan Zhao <yuanyuan.zhao@...> writes:

For TSC's precision (20~100ppm) is lower than physical RTC (less than 20ppm),
vRTC need to be calibrated by physical RTC. A timer tiggers calibration for
vRTC every 3 hours. This can improve efficiency because physical RTC can be
read once and calibrate all vRTC.

Signed-off-by: Yuanyuan Zhao <yuanyuan.zhao@...>
---
hypervisor/dm/vrtc.c | 80 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 77 insertions(+), 3 deletions(-)

diff --git a/hypervisor/dm/vrtc.c b/hypervisor/dm/vrtc.c
index ba0f510e2..ee9b47087 100644
--- a/hypervisor/dm/vrtc.c
+++ b/hypervisor/dm/vrtc.c
@@ -51,6 +51,8 @@ struct clktime {
uint32_t dow; /* day of week (0 - 6; 0 = Sunday) */
};
+static spinlock_t calibrate_lock = { .head = 0U, .tail = 0U };
`Calibrate` is a verb and thus makes `calibrate lock` misleading
(because you are not calibrate a lock).
What's the complete list of variables you want to protect using this
lock?

+
#define POSIX_BASE_YEAR 1970
#define SECDAY (24 * 60 * 60)
#define SECYR (SECDAY * 365)
@@ -381,10 +383,12 @@ static time_t vrtc_get_current_time(struct acrn_vrtc *vrtc)
uint64_t offset;
time_t second = VRTC_BROKEN_TIME;
+ spinlock_obtain(&calibrate_lock);
if (vrtc->base_rtctime > 0) {
offset = (cpu_ticks() - vrtc->base_tsc) / (get_tsc_khz() * 1000U);
second = vrtc->base_rtctime + vrtc->offset_rtctime + (time_t)offset;
}
+ spinlock_release(&calibrate_lock);
return second;
}
@@ -564,7 +568,9 @@ static bool vrtc_write(struct acrn_vcpu *vcpu, uint16_t
addr, size_t width,
*((uint8_t *)&vrtc->rtcdev + vrtc->addr) = (uint8_t)(value & 0xFFU);
current = vrtc_get_current_time(vrtc);
after = rtc_to_secs(vrtc);
+ spinlock_obtain(&calibrate_lock);
vrtc->offset_rtctime += after - current;
+ spinlock_release(&calibrate_lock);
break;
}
}
@@ -573,9 +579,70 @@ static bool vrtc_write(struct acrn_vcpu *vcpu, uint16_t addr, size_t width,
return true;
}
+#define CALIBRATE_PERIOD (3 * 3600 * 1000) /* By ms, totally 3
hours. */
+static struct hv_timer calibrate_timer;
+
+static time_t vrtc_get_physical_rtc_time(struct acrn_vrtc *vrtc)
+{
+ uint8_t *rtc = (uint8_t *)&vrtc->rtcdev;
+
+ *(rtc + RTC_SEC) = cmos_get_reg_val(RTC_SEC);
+ *(rtc + RTC_MIN) = cmos_get_reg_val(RTC_MIN);
+ *(rtc + RTC_HRS) = cmos_get_reg_val(RTC_HRS);
+ *(rtc + RTC_DAY) = cmos_get_reg_val(RTC_DAY);
+ *(rtc + RTC_MONTH) = cmos_get_reg_val(RTC_MONTH);
+ *(rtc + RTC_YEAR) = cmos_get_reg_val(RTC_YEAR);
+ *(rtc + RTC_CENTURY) = cmos_get_reg_val(RTC_CENTURY);
+ *(rtc + RTC_STATUSB) = cmos_get_reg_val(RTC_STATUSB);
+
+ return rtc_to_secs(vrtc);
+}
+
+static void vrtc_update_basetime(time_t physical_time, time_t offset)
+{
+ struct acrn_vm *vm;
+ uint32_t vm_id;
+
+ for (vm_id = 0U; vm_id < CONFIG_MAX_VM_NUM; vm_id++) {
+ vm = get_vm_from_vmid(vm_id);
+ if (is_rt_vm(vm) || is_prelaunched_vm(vm)) {
+ spinlock_obtain(&calibrate_lock);
+ vm->vrtc.base_tsc = cpu_ticks();
+ vm->vrtc.base_rtctime = physical_time;
+ vm->vrtc.offset_rtctime += offset;
I don't see any change to ensure monotonicity. Have I missed anything?
Monotonicity ensured by calibration period:


For TSC's precision (20~100ppm) is lower than physical RTC (less than
20ppm),
vRTC need to be calibrated by physical RTC. A timer tiggers calibration for
vRTC every 3 hours. This can improve efficiency because physical RTC can be
read once and calibrate all vRTC.
How is monotonicity derived from the method above?
1. Given the parameters above, the wrost-case difference between two
clocks can be 120ppm, which is ~1.3s.
2. Even the rebasing is done more frequently to control the worst-case
difference, there's still a possibility that the vRTC is rebased like
the following:
1ms later, -10ms
12:30:58 --> 12:30:59 ----------------> rebased to 12:30:58
system time: 12:30:59 ----------------> get rtc time 12:30:58

---> wait alarm interrupt 12:30:59 ----> system time 10:30.59
There is no restriction that RTC must be read after an alarm interrupt.

--
Best Regards
Junjie Mao


Re: [PATCH v2] misc: configurator: Config usb mediator devices in dropdown list

Junjie Mao
 

Calvin Zhang <calvinzhang.cool@...> writes:

Support detecting connected usb devices in board_inspector and list them
in the usb mediator configuration menu.

Tracked-On: #7424
Signed-off-by: Calvin Zhang <calvinzhang.cool@...>
Reviewed-by: Junjie Mao <junjie.mao@...>

Some minor comments below.

---

Notes:
v2: move usb_device node under the acpi port device

.../board_inspector/extractors/95-usb.py | 38 +++++++++++++++++++
.../launch_config/launch_cfg_gen.py | 5 ++-
misc/config_tools/schema/VMtypes.xsd | 12 ++++++
misc/config_tools/schema/config.xsd | 10 +----
4 files changed, 54 insertions(+), 11 deletions(-)
create mode 100644 misc/config_tools/board_inspector/extractors/95-usb.py

diff --git a/misc/config_tools/board_inspector/extractors/95-usb.py b/misc/config_tools/board_inspector/extractors/95-usb.py
new file mode 100644
index 000000000..90ddac53c
--- /dev/null
+++ b/misc/config_tools/board_inspector/extractors/95-usb.py
@@ -0,0 +1,38 @@
+# Copyright (C) 2022 Intel Corporation. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-3-Clause
+#
+
+import os, re
+
+from extractors.helpers import add_child, get_node
+
+USB_DEVICES_PATH = "/sys/bus/usb/devices"
+USB_DEVICES_REGEX = r"^\d-\d$" # only devices connecting to root hub
+
+def extract(args, board_etree):
+ usb_devices_node = get_node(board_etree, f"//usb_devices")
+ if not usb_devices_node:
+ devices_node = get_node(board_etree, f"//devices")
+ usb_devices_node = add_child(devices_node, "usb_devices")
Drop the code above.

+
+ dev_regex = re.compile(USB_DEVICES_REGEX)
+ for dev in os.listdir(USB_DEVICES_PATH):
+ m = dev_regex.match(dev)
+ if m:
+ d = m.group(0)
+ devpath = os.path.join(USB_DEVICES_PATH, d)
+ with open(os.path.join(devpath, 'devnum'), 'r') as f:
+ devnum = f.read().strip()
+ with open(os.path.join(devpath, 'busnum'), 'r') as f:
+ busnum = f.read().strip()
+ cmd_out = os.popen('lsusb -s {b}:{d}'.format(b=busnum, d=devnum)).read()
+ desc = cmd_out.split(':', maxsplit=1)[1].strip('\n')
+
+ with open(devpath + '/port/firmware_node/path') as f:
+ acpi_path = f.read().strip()
+ usb_port_node = get_node(board_etree, f"//device[acpi_object='{acpi_path}']")
+ if usb_port_node is not None:
+ add_child(usb_port_node, "usb_device", location=d,
+ description=d + desc)
+
diff --git a/misc/config_tools/launch_config/launch_cfg_gen.py b/misc/config_tools/launch_config/launch_cfg_gen.py
index 302918e06..a168182e6 100755
--- a/misc/config_tools/launch_config/launch_cfg_gen.py
+++ b/misc/config_tools/launch_config/launch_cfg_gen.py
@@ -267,8 +267,9 @@ def generate_for_one_vm(board_etree, hv_scenario_etree, vm_scenario_etree, vm_id
script.add_virtual_device("uart", options="vuart_idx:{idx}")

# Mediated PCI devices, including virtio
- for usb_xhci in eval_xpath_all(vm_scenario_etree, ".//usb_xhci[text() != '']/text()"):
- script.add_virtual_device("xhci", options=usb_xhci)
+ for usb_xhci in eval_xpath_all(vm_scenario_etree, ".//usb_xhci/usb_dev[text() != '']/text()"):
+ bus_port = usb_xhci.split(' ')[0]
+ script.add_virtual_device("xhci", options=bus_port)

for virtio_input_etree in eval_xpath_all(vm_scenario_etree, ".//virtio_devices/input"):
backend_device_file = eval_xpath(virtio_input_etree, "./backend_device_file[text() != '']/text()")
diff --git a/misc/config_tools/schema/VMtypes.xsd b/misc/config_tools/schema/VMtypes.xsd
index f98c068d4..e03eb61a3 100644
--- a/misc/config_tools/schema/VMtypes.xsd
+++ b/misc/config_tools/schema/VMtypes.xsd
@@ -284,6 +284,18 @@ The size is a subset of the VM's total memory size specified on the Basic tab.</
</xs:sequence>
</xs:complexType>

+<xs:complexType name="USBDevsConfiguration">
+ <xs:sequence>
+ <xs:element name="usb_dev" type="xs:string" minOccurs="0" maxOccurs="unbounded">
+ <xs:annotation acrn:title="USB device assignment"
+ acrn:options="//usb_device/@description" acrn:options-sorted-by="lambda s: (s, s.split(' ')[0])">
If two descriptions are exactly the same, `s.split(' ')[0]`, which is
part of the description, will be as well. Why do we want to add this
second component then?

If you want to sort by the descriptions themselves, using `lambda s: s`,
i.e. an identity function.

+ <xs:documentation>Select the USB devices you want to assign to this virtual machine.</xs:documentation>
+ </xs:annotation>
+ </xs:element>
+ </xs:sequence>
+</xs:complexType>
+
+
<xs:simpleType name="VirtioNetworkFrameworkType">
<xs:annotation>
<xs:documentation>A string with value: ``Kernel based (Virtual Host)`` or ``User space based (VBSU)``.</xs:documentation>
diff --git a/misc/config_tools/schema/config.xsd b/misc/config_tools/schema/config.xsd
index 68cc575c6..2669b2a73 100644
--- a/misc/config_tools/schema/config.xsd
+++ b/misc/config_tools/schema/config.xsd
@@ -433,18 +433,10 @@ argument and memory.</xs:documentation>
<xs:documentation>Enable the ACRN Device Model to emulate COM1 as a User VM stdio I/O. Hypervisor global emulation will take priority over this VM setting.</xs:documentation>
</xs:annotation>
</xs:element>
- <xs:element name="usb_xhci" minOccurs="0">
+ <xs:element name="usb_xhci" type="USBDevsConfiguration" minOccurs="0">
<xs:annotation acrn:title="Virtual USB HCI" acrn:views="basic" acrn:applicable-vms="post-launched">
<xs:documentation>Select the USB physical bus and port number that will be emulated by the ACRN Device Model for this VM. USB 3.0, 2.0, and 1.0 are supported.</xs:documentation>
</xs:annotation>
- <xs:simpleType>
- <xs:annotation>
- <xs:documentation>Input format: ``bus#-port#[:bus#-port#: ...]``, for example, ``1-2:2-4``.</xs:documentation>
- </xs:annotation>
- <xs:restriction base="xs:string">
- <xs:pattern value="([\d]+-[\d]+){0,1}(:[\d]+-[\d]+)*" />
- </xs:restriction>
- </xs:simpleType>
</xs:element>
<xs:element name="virtio_devices">
<xs:annotation acrn:title="Virt-IO devices" acrn:views="basic" acrn:applicable-vms="post-launched">
--
Best Regards
Junjie Mao


Re: [PATCH v2 1/2] misc: refine CLOS module for expansibility

chenli.wei
 

On 5/7/2022 10:38 AM, Junjie Mao wrote:
Chenli Wei <chenli.wei@...> writes:

This patch refine CLOS module by the following aspects:

1 Unified CACHE_ID type to Hex Format
2 Rewrite policy merge with RDT Class
3 Modify the logic of generate CPU mask

v1-->v2:
1. code format
2. add comment for RdtPolicy

Signed-off-by: Chenli Wei <chenli.wei@...>
---
misc/config_tools/board_config/board_c.py | 8 +-
misc/config_tools/schema/types.xsd | 2 +-
misc/config_tools/static_allocators/clos.py | 233 ++++++++++++--------
3 files changed, 144 insertions(+), 99 deletions(-)

diff --git a/misc/config_tools/board_config/board_c.py b/misc/config_tools/board_config/board_c.py
index 8082c4387..4be3ea4ec 100644
--- a/misc/config_tools/board_config/board_c.py
+++ b/misc/config_tools/board_config/board_c.py
@@ -173,7 +173,7 @@ def gen_rdt_str(cache, config):
return err_dic

cdp_enable = get_cdp_enabled()
- cat_mask_list = get_mask_list(cache_level, int(cache_id, 16))
+ cat_mask_list = get_mask_list(cache_level, cache_id)
if len(cat_mask_list) > int(clos_number):
err_dic['board config: Failed to generate board.c'] = "CLOS Mask Number too bigger then the supported of L2/L3 cache"
return err_dic;
@@ -207,7 +207,7 @@ def gen_rdt_str(cache, config):

cpu_mask = 0
for processor in processor_list:
- core_id = common.get_node(f"//core[@id = '{processor}']/thread/cpu_id/text()", board_etree)
+ core_id = common.get_node(f"//thread[apic_id = '{processor}']/cpu_id/text()", board_etree)
if core_id is None:
continue
else:
@@ -240,7 +240,7 @@ def gen_clos_array(cache_list, config):
clos_number = common.get_node(f"./capability/clos_number/text()", cache)
if cache_level == "2":

- cat_mask_list = get_mask_list(cache_level, int(cache_id, 16))
+ cat_mask_list = get_mask_list(cache_level, cache_id)
array_size = len(cat_mask_list)

print("union clos_config platform_l2_clos_array_{0}[{1}] = {{".format(int(cache_id, 16), clos_number), file=config)
@@ -250,7 +250,7 @@ def gen_clos_array(cache_list, config):
print("};\n", file=config)
res_present[RDT.L2.value] += 1
elif cache_level == "3":
- cat_mask_list = get_mask_list(cache_level, int(cache_id, 16))
+ cat_mask_list = get_mask_list(cache_level, cache_id)

print("union clos_config platform_l3_clos_array_{0}[{1}] = {{".format(int(cache_id, 16), clos_number), file=config)

diff --git a/misc/config_tools/schema/types.xsd b/misc/config_tools/schema/types.xsd
index 36a4aae09..cec4b84a5 100644
--- a/misc/config_tools/schema/types.xsd
+++ b/misc/config_tools/schema/types.xsd
@@ -355,7 +355,7 @@ RDT, setting this option to ``y`` is ignored.</xs:documentation>

<xs:complexType name="CacheAllocationType">
<xs:sequence>
- <xs:element name="CACHE_ID" type="xs:integer"/>
+ <xs:element name="CACHE_ID" type="HexFormat"/>
<xs:element name="CACHE_LEVEL" type="xs:integer"/>
<xs:element name="POLICY" type="CachePolicyType" minOccurs="1" maxOccurs="unbounded"/>
</xs:sequence>
diff --git a/misc/config_tools/static_allocators/clos.py b/misc/config_tools/static_allocators/clos.py
index 1f0476422..80b59435d 100644
--- a/misc/config_tools/static_allocators/clos.py
+++ b/misc/config_tools/static_allocators/clos.py
@@ -12,114 +12,159 @@ import re
from collections import defaultdict
from itertools import combinations

-def create_clos_node(etree, vm_id, index_list):
- allocation_vm_node = common.get_node(f"/acrn-config/vm[@id = '{vm_id}']", etree)
+class Identifier:
Use `namedtuple`. No need to write 10 lines only to revent a wheel.
OK

Also, this class represents an identifier of what? Have you considered
CDP here as well?
The "vm_name", "vcpu" and "cache_type" could identifier a policy.

The "cache_type " could be "Code" and "Data" if we enable CDP mode.


+ vm_name = ""
+ vcpu = ""
+ cache_type = ""
+
+ def __init__(self, vm_name, vcpu, cache_type):
+ self.vm_name = vm_name
+ self.vcpu = vcpu
+ self.cache_type = cache_type
+
+ def __eq__(self, other):
+ return (self.vm_name == other.vm_name) and (self.vcpu == other.vcpu) and (self.cache_type == other.cache_type)
+
+class RdtPolicy:
+
+# a dictionary to save the CLOS policy from user setting
Indent the comments to the same level as the code you are commenting on.
OK

+ policy_dict = {}
+
+# a list stored the vCPU or VM info
+ policy_owner = []
+
+# a list stored the L2 cache IDs
+ cache2_id_list = []
+
+#L2 cache have more then one section, this function find which one have been set
Typo: then -> than
Done

+ def find_cache2_id(self, mask):
+ for cache2 in self.cache2_id_list:
+ if mask[cache2] != None:
+ return cache2
+ return None
+
+ def __init__(self, policy_dict, owner):
+ self.policy_dict = policy_dict
+ self.policy_owner = [owner]
+
+#check whether the src could be merged, if yes, add the src owner to policy_owner list and return True
+ def merge_policy(self, src):
+ if self.policy_dict["l3"] == src.policy_dict["l3"]:
While dicts in Python can use arbitrary keys (even with different
types), the code is much less maintainable if you do so.

If you want to wrap RDT policies as objects, do the following:

1. Instead of using an all-in-one dict, split L2 and L3 configurations
as different fields.
OK, I will split it.

2. Add an interface that takes an XML element and updates the objects,
so that when walking through the policies you do not need to create
any more dictionaries or lists as another level of intermediate
representation.
OK, I will try to add new interface.

+ cache2_id = self.find_cache2_id(src.policy_dict)
+ if (cache2_id == None) or (self.policy_dict[cache2_id] == src.policy_dict[cache2_id]):
+ self.policy_owner.append(src.policy_owner[0])
In theory you should extend `self.policy_owner` using
`src.policy_owner`. While today `src` may have at most one owner, it
relies on how you walk through the policies which this class had better
not make any assumption on.
Yes, you are right, I will use a loop to append the src.policy_owner list.


+ return True
+ elif self.policy_dict[cache2_id] == None:
+ self.policy_dict[cache2_id] = src.policy_dict[cache2_id]
+ return True
+ return False
+
+#check whether a VM/vCPU could adapt this policy
+ def adapt_policy(self, policy_identifier):
"adapt" means "make sth. suitable", but this function is only a
predicate with no side effects.
Use "find" to replace it.
+ for owner in self.policy_owner:
+ if owner == policy_identifier:
+ return True
+ return False
Do you mean `return policy_identifier in self.policy_owner` by the above
four lines?
Yes.

policy_identifier is an object, so I use the Operator overloading "__eq__"
These code could by simple after use "namedtuple".


+
+class vCatPolicy(RdtPolicy):
+ def merge_policy(self, src):
+ return False
+
+def create_clos_node(scenario_etree, vm_id, index_list):
+ allocation_vm_node = common.get_node(f"/acrn-config/vm[@id = '{vm_id}']", scenario_etree)
if allocation_vm_node is None:
- allocation_vm_node = common.append_node("/acrn-config/vm", None, etree, id = vm_id)
+ allocation_vm_node = common.append_node("/acrn-config/vm", None, scenario_etree, id = vm_id)
if common.get_node("./clos", allocation_vm_node) is None:
clos_node = common.append_node("./clos", None, allocation_vm_node)
for index in index_list:
common.append_node(f"./vcpu_clos", str(index), clos_node)

-def find_cache2_id(mask, cache2_id_list):
- for cache2 in cache2_id_list:
- if mask[cache2] != "None":
- return cache2
- return "None"
-
-def merge_policy_list(mask_list, cache2_id_list):
- index = 0
+def merge_policy_list(policy_list):
result_list = []
- for index,mask in enumerate(mask_list):
- merged = 0
+ for index,p in enumerate(policy_list):
+ merged = False
if index == 0:
Why do need this branch? When `index` is 0, `result_list` is empty and
`merged` will always be `False` after you walk through that empty
list. Adding this specific branch does little (if any) help to
performance but hurts a lot in readability.
I will remove this branch.

- result_list.append(mask)
+ result_list.append(p)
continue
for result in result_list:
- if result["l3"] != mask["l3"]:
- continue
- else:
- cache2_id = find_cache2_id(mask, cache2_id_list)
- if cache2_id == "None" or result[cache2_id] == mask[cache2_id]:
- merged = 1
- break
- if result[cache2_id] == "None":
- merged = 1
- result[cache2_id] = mask[cache2_id]
- break
- if merged == 0:
- result_list.append(mask)
+ if result.merge_policy(p):
+ merged = True
+ break;
+ if merged == False:
Simply say `if not merged`.
OK

+ result_list.append(p)
return result_list

-def gen_all_clos_index(board_etree, scenario_etree, allocation_etree):
- policy_list = []
- allocation_list = scenario_etree.xpath(f"//POLICY")
+def gen_identifier_list(scenario_etree):
+ identifier_list = []
+ vm_list = scenario_etree.xpath("//POLICY/VM")
+ for vm in vm_list:
+ vm_name = common.get_node("./text()", vm)
+ vcpu = common.get_node("../VCPU/text()", vm)
+ cache_type = common.get_node("../TYPE/text()", vm)
+ identifier_list.append(Identifier(vm_name, vcpu, cache_type))
+ return identifier_list
+
+def vm_vcat_enable(scenario_etree, vm_name):
+ vcat_enable = common.get_node(f"//VCAT_ENABLED/text()", scenario_etree)
+ virtual_cat_support = common.get_node(f"//vm[name = '{vm_name}']/virtual_cat_support/text()", scenario_etree)
+ return (vcat_enable == "y") and (virtual_cat_support == "y")
+
+def get_policy_list(board_etree, scenario_etree, allocation_etree):
cache2_id_list = scenario_etree.xpath("//CACHE_ALLOCATION[CACHE_LEVEL = 2]/CACHE_ID/text()")
cache2_id_list.sort()

- for policy in allocation_list:
- cache_level = common.get_node("../CACHE_LEVEL/text()", policy)
- cache_id = common.get_node("../CACHE_ID/text()", policy)
- vcpu = common.get_node("./VCPU/text()", policy)
- mask = common.get_node("./CLOS_MASK/text()", policy)
- tmp = (cache_level, cache_id, vcpu, mask)
- policy_list.append(tmp)
-
- vCPU_list = scenario_etree.xpath(f"//POLICY/VCPU/text()")
- l3_mask_list = scenario_etree.xpath(f"//CACHE_ALLOCATION[CACHE_LEVEL = 3]/POLICY/CLOS_MASK")
- mask_list = []
- for vCPU in vCPU_list:
+ RdtPolicy.cache2_id_list = cache2_id_list
+ identifier_list = gen_identifier_list(scenario_etree)
+
+ result_list = []
+ for ident in identifier_list:
dict_tmp = {}
- l3_mask = l2_mask = "None"
- l3_mask_list = scenario_etree.xpath(f"//CACHE_ALLOCATION[CACHE_LEVEL = 3]/POLICY[VCPU = '{vCPU}']/CLOS_MASK/text()")
- if len(l3_mask_list) > 0:
- l3_mask = l3_mask_list[0]
+ policy_list = scenario_etree.xpath(f"//POLICY[VM = '{ident.vm_name}' and VCPU = '{ident.vcpu}' and TYPE = '{ident.cache_type}']")
+ l3_mask = l2_mask = None
+ cache2_id = None
+ for policy in policy_list:
+ cache_level = common.get_node("../CACHE_LEVEL/text()", policy)
+ cache_id = common.get_node("../CACHE_ID/text()", policy)
+ clos_mask = common.get_node("./CLOS_MASK/text()", policy)
+ if cache_level == "2":
+ l2_mask = clos_mask
+ cache2_id = cache_id
+ else:
+ l3_mask = clos_mask
dict_tmp["l3"] = l3_mask
-
- l2_mask_list = scenario_etree.xpath(f"//CACHE_ALLOCATION[CACHE_LEVEL = 2]/POLICY[VCPU = '{vCPU}']/CLOS_MASK")
- if len(l2_mask_list) > 0:
- l2_mask = l2_mask_list[0].text
- cache_id = scenario_etree.xpath(f"//CACHE_ALLOCATION[CACHE_LEVEL = 2 and POLICY/VCPU = '{vCPU}']/CACHE_ID/text()")[0]
for cache2 in cache2_id_list:
- if cache2 == cache_id:
+ if cache2 == cache2_id:
dict_tmp[cache_id] = l2_mask
else:
- dict_tmp[cache2] = "None"
- mask_list.append(dict_tmp)
- mask_list = merge_policy_list(mask_list, cache2_id_list)
- return mask_list
-
-def get_clos_index(cache_level, cache_id, clos_mask):
- mask_list = common.get_mask_list(cache_level, cache_id)
- idx = 0
- for mask in mask_list:
- idx += 1
- if mask == clos_mask:
- break
- return idx
-def get_clos_id(mask_list, l2_id, l2_mask, l3_mask):
- for mask in mask_list:
- if mask[l2_id] == l2_mask and mask["l3"] == l3_mask:
- return mask_list.index(mask)
+ dict_tmp[cache2] = None
+ if vm_vcat_enable(scenario_etree, ident.vm_name):
+ result_list.append(vCatPolicy(dict_tmp, ident))
+ else:
+ result_list.append(RdtPolicy(dict_tmp, ident))
+ return merge_policy_list(result_list)
+
+def get_clos_id(rdt_list, mask_identifier):
+ for index,rdt in enumerate(rdt_list):
+ if rdt.adapt_policy(mask_identifier):
Please, be consistent with your names. What's the difference between
"identifier" and "owner"? If they are the same, unify the noun you use
to refer to them; otherwise, why you compare identifiers with owners in
the `adapt_policy` member function?

I'll stop here, as this patch still need a fundamental refactoring to
make the overall structure easy to read.
Very thanks for your review, I will refine again.

--
Best Regards
Junjie Mao

+ return index
return 0

def alloc_clos_index(board_etree, scenario_etree, allocation_etree, mask_list):
vm_node_list = scenario_etree.xpath("//vm")
for vm_node in vm_node_list:
- vmname = common.get_node("./name/text()", vm_node)
- allocation_list = scenario_etree.xpath(f"//CACHE_ALLOCATION[POLICY/VM = '{vmname}']")
- for allocation in allocation_list:
- index_list = []
- cache_level = common.get_node("./CACHE_LEVEL/text()", allocation)
- cache_id = common.get_node("./CACHE_ID/text()", allocation)
- clos_mask_list = allocation.xpath(f".//POLICY[VM = '{vmname}']/CLOS_MASK/text()")
-
- for clos_mask in clos_mask_list:
- index = get_clos_id(mask_list, cache_id, clos_mask, "None")
+ vm_name = common.get_node("./name/text()", vm_node)
+ vcpu_list = scenario_etree.xpath(f"//POLICY[VM = '{vm_name}']/VCPU/text()")
+ index_list = []
+ for vcpu in sorted(list(set(vcpu_list))):
+ type_list = scenario_etree.xpath(f"//POLICY[VM = '{vm_name}' and VCPU = '{vcpu}']/TYPE/text()")
+ for cache_type in sorted(list(set(type_list))):
+ if cache_type == "Data":
+ continue
+ index = get_clos_id(mask_list, Identifier(vm_name, vcpu, cache_type))
index_list.append(index)
- create_clos_node(allocation_etree, common.get_node("./@id", vm_node), index_list)
+ create_clos_node(allocation_etree, common.get_node("./@id", vm_node), index_list)

-def creat_mask_list_node(board_etree, scenario_etree, allocation_etree, mask_list):
+def create_mask_list_node(board_etree, scenario_etree, allocation_etree, rdt_policy_list):
allocation_hv_node = common.get_node(f"//hv", allocation_etree)
if allocation_hv_node is None:
allocation_hv_node = common.append_node(f"/acrn-config/hv", None, allocation_etree)
@@ -127,24 +172,24 @@ def creat_mask_list_node(board_etree, scenario_etree, allocation_etree, mask_lis
cache2_id_list.sort()
if common.get_node("./clos_mask[@id = l3]", allocation_hv_node) is None:
clos_mask = common.append_node("./clos_mask", None, allocation_hv_node, id="l3")
- for i in range(0, len(mask_list)):
- if mask_list[i]["l3"] == "None":
- value = "0xffff"
- else:
- value = str(mask_list[i]["l3"])
+ length = common.get_node(f"//cache[@level='3']/capability/capacity_mask_length/text()", board_etree)
+ value = hex((1 << int(length)) - 1)
+ for i in range(0, len(rdt_policy_list)):
+ if rdt_policy_list[i].policy_dict["l3"] is not None:
+ value = str(rdt_policy_list[i].policy_dict["l3"])
common.append_node(f"./clos", value, clos_mask)

for cache2 in cache2_id_list:
+ length = common.get_node(f"//cache[@level='2' and @id = '{cache2}']/capability/capacity_mask_length/text()", board_etree)
+ value = hex((1 << int(length)) - 1)
if common.get_node("./clos_mask[@id = '{cache2}']", allocation_hv_node) is None:
clos_mask = common.append_node("./clos_mask", None, allocation_hv_node, id=cache2)
- for i in range(0, len(mask_list)):
- if mask_list[i][cache2] == "None":
- value = "0xffff"
- else:
- value = str(mask_list[i][cache2] )
+ for i in range(0, len(rdt_policy_list)):
+ if rdt_policy_list[i].policy_dict[cache2] is not None:
+ value = str(rdt_policy_list[i].policy_dict[cache2] )
common.append_node(f"./clos", value, clos_mask)

def fn(board_etree, scenario_etree, allocation_etree):
- mask_list = gen_all_clos_index(board_etree, scenario_etree, allocation_etree)
- creat_mask_list_node(board_etree, scenario_etree, allocation_etree, mask_list)
- alloc_clos_index(board_etree, scenario_etree, allocation_etree, mask_list)
+ policy_list = get_policy_list(board_etree, scenario_etree, allocation_etree)
+ create_mask_list_node(board_etree, scenario_etree, allocation_etree, policy_list)
+ alloc_clos_index(board_etree, scenario_etree, allocation_etree, policy_list)


[PATCH v5 8/8] hv: calibrate vrtc when modify physical time

Zhao, Yuanyuan
 

For Service VM modify physical rtc time and vrtc will calibrate
time by read physical rtc. So when Service VM modify physical
time, calibrate all vrtc.

Reviewed-by: Junjie Mao <junjie.mao@...>
Signed-off-by: Yuanyuan Zhao <yuanyuan.zhao@...>
---
hypervisor/dm/vrtc.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/hypervisor/dm/vrtc.c b/hypervisor/dm/vrtc.c
index 2231e31ca..e27fe0909 100644
--- a/hypervisor/dm/vrtc.c
+++ b/hypervisor/dm/vrtc.c
@@ -41,6 +41,9 @@
#endif
# define RTC_ERR pr_err

+static time_t vrtc_get_physical_rtc_time(struct acrn_vrtc *vrtc);
+static void vrtc_update_basetime(time_t physical_time, time_t offset);
+
struct clktime {
uint32_t year; /* year (4 digit year) */
uint32_t mon; /* month (1 - 12) */
@@ -524,6 +527,12 @@ static bool vrtc_read(struct acrn_vcpu *vcpu, uint16_t addr, __unused size_t wid
return ret;
}

+static inline bool vrtc_is_time_register(uint32_t offset)
+{
+ return (offset == RTC_SEC || offset == RTC_MIN || offset == RTC_HRS || offset == RTC_DAY
+ || offset == RTC_MONTH || offset == RTC_YEAR || offset == RTC_CENTURY);
+}
+
/**
* @pre vcpu != NULL
* @pre vcpu->vm != NULL
@@ -533,12 +542,22 @@ static bool vrtc_write(struct acrn_vcpu *vcpu, uint16_t addr, size_t width,
{
time_t current, after;
struct acrn_vrtc *vrtc = &vcpu->vm->vrtc;
+ struct acrn_vrtc temp_vrtc;
+ bool is_time_register;

if ((width == 1U) && (addr == CMOS_ADDR_PORT)) {
vrtc->addr = (uint8_t)(value & 0x7FU);
} else {
if (is_service_vm(vcpu->vm)) {
- cmos_set_reg_val(vrtc->addr, (uint8_t)(value & 0xFFU));
+ is_time_register = vrtc_is_time_register(vrtc->addr);
+ if (is_time_register) {
+ current = vrtc_get_physical_rtc_time(&temp_vrtc);
+ }
+ cmos_set_reg_val(vcpu->vm->vrtc.addr, (uint8_t)(value & 0xFFU));
+ if (is_time_register) {
+ after = vrtc_get_physical_rtc_time(&temp_vrtc);
+ vrtc_update_basetime(after, current - after);
+ }
} else {
switch (vrtc->addr) {
case RTC_STATUSA:
--
2.25.1


[PATCH v5 7/8] hv: vrtc calibrate function

Zhao, Yuanyuan
 

For TSC's precision (20~100ppm) is lower than physical RTC (less than 20ppm),
vRTC need to be calibrated by physical RTC. A timer tiggers calibration for
vRTC every 3 hours. This can improve efficiency because physical RTC can be
read once and calibrate all vRTC.

Signed-off-by: Yuanyuan Zhao <yuanyuan.zhao@...>
---
hypervisor/dm/vrtc.c | 80 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 77 insertions(+), 3 deletions(-)

diff --git a/hypervisor/dm/vrtc.c b/hypervisor/dm/vrtc.c
index 4a404cb69..2231e31ca 100644
--- a/hypervisor/dm/vrtc.c
+++ b/hypervisor/dm/vrtc.c
@@ -51,6 +51,8 @@ struct clktime {
uint32_t dow; /* day of week (0 - 6; 0 = Sunday) */
};

+static spinlock_t vrtc_rebase_lock = { .head = 0U, .tail = 0U };
+
#define POSIX_BASE_YEAR 1970
#define SECDAY (24 * 60 * 60)
#define SECYR (SECDAY * 365)
@@ -381,10 +383,12 @@ static time_t vrtc_get_current_time(struct acrn_vrtc *vrtc)
uint64_t offset;
time_t second = VRTC_BROKEN_TIME;

+ spinlock_obtain(&vrtc_rebase_lock);
if (vrtc->base_rtctime > 0) {
offset = (cpu_ticks() - vrtc->base_tsc) / (get_tsc_khz() * 1000U);
second = vrtc->base_rtctime + vrtc->offset_rtctime + (time_t)offset;
}
+ spinlock_release(&vrtc_rebase_lock);
return second;
}

@@ -564,7 +568,9 @@ static bool vrtc_write(struct acrn_vcpu *vcpu, uint16_t addr, size_t width,
*((uint8_t *)&vrtc->rtcdev + vrtc->addr) = (uint8_t)(value & 0xFFU);
current = vrtc_get_current_time(vrtc);
after = rtc_to_secs(vrtc);
+ spinlock_obtain(&vrtc_rebase_lock);
vrtc->offset_rtctime += after - current;
+ spinlock_release(&vrtc_rebase_lock);
break;
}
}
@@ -573,9 +579,70 @@ static bool vrtc_write(struct acrn_vcpu *vcpu, uint16_t addr, size_t width,
return true;
}

+#define CALIBRATE_PERIOD (3 * 3600 * 1000) /* By ms, totally 3 hours. */
+static struct hv_timer calibrate_timer;
+
+static time_t vrtc_get_physical_rtc_time(struct acrn_vrtc *vrtc)
+{
+ uint8_t *rtc = (uint8_t *)&vrtc->rtcdev;
+
+ *(rtc + RTC_SEC) = cmos_get_reg_val(RTC_SEC);
+ *(rtc + RTC_MIN) = cmos_get_reg_val(RTC_MIN);
+ *(rtc + RTC_HRS) = cmos_get_reg_val(RTC_HRS);
+ *(rtc + RTC_DAY) = cmos_get_reg_val(RTC_DAY);
+ *(rtc + RTC_MONTH) = cmos_get_reg_val(RTC_MONTH);
+ *(rtc + RTC_YEAR) = cmos_get_reg_val(RTC_YEAR);
+ *(rtc + RTC_CENTURY) = cmos_get_reg_val(RTC_CENTURY);
+ *(rtc + RTC_STATUSB) = cmos_get_reg_val(RTC_STATUSB);
+
+ return rtc_to_secs(vrtc);
+}
+
+static void vrtc_update_basetime(time_t physical_time, time_t offset)
+{
+ struct acrn_vm *vm;
+ uint32_t vm_id;
+
+ for (vm_id = 0U; vm_id < CONFIG_MAX_VM_NUM; vm_id++) {
+ vm = get_vm_from_vmid(vm_id);
+ if (is_rt_vm(vm) || is_prelaunched_vm(vm)) {
+ spinlock_obtain(&vrtc_rebase_lock);
+ vm->vrtc.base_tsc = cpu_ticks();
+ vm->vrtc.base_rtctime = physical_time;
+ vm->vrtc.offset_rtctime += offset;
+ spinlock_release(&vrtc_rebase_lock);
+ }
+ }
+}
+
+static void calibrate_timer_callback(__unused void *data)
+{
+ struct acrn_vrtc temp_vrtc;
+ time_t physical_time = vrtc_get_physical_rtc_time(&temp_vrtc);
+
+ vrtc_update_basetime(physical_time, 0);
+}
+
+static void calibrate_setup_timer(void)
+{
+ uint64_t period_in_cycle, fire_tsc;
+
+ period_in_cycle = TICKS_PER_MS * CALIBRATE_PERIOD;
+ fire_tsc = cpu_ticks() + period_in_cycle;
+ initialize_timer(&calibrate_timer,
+ calibrate_timer_callback, NULL,
+ fire_tsc, period_in_cycle);
+
+ /* Start an periodic timer */
+ if (add_timer(&calibrate_timer) != 0) {
+ pr_err("Failed to add calibrate timer");
+ }
+}
+
static void vrtc_set_basetime(struct acrn_vrtc *vrtc)
{
struct rtcdev *vrtcdev = &vrtc->rtcdev;
+ time_t current;

/*
* Read base time from physical rtc.
@@ -592,7 +659,10 @@ static void vrtc_set_basetime(struct acrn_vrtc *vrtc)
vrtcdev->reg_c = cmos_get_reg_val(RTC_INTR);
vrtcdev->reg_d = cmos_get_reg_val(RTC_STATUSD);

- vrtc->base_rtctime = rtc_to_secs(vrtc);
+ current = rtc_to_secs(vrtc);
+ spinlock_obtain(&vrtc_rebase_lock);
+ vrtc->base_rtctime = current;
+ spinlock_release(&vrtc_rebase_lock);
}

void vrtc_init(struct acrn_vm *vm)
@@ -606,6 +676,10 @@ void vrtc_init(struct acrn_vm *vm)
vm->vrtc.vm = vm;
register_pio_emulation_handler(vm, RTC_PIO_IDX, &range, vrtc_read, vrtc_write);

- vrtc_set_basetime(&vm->vrtc);
- vm->vrtc.base_tsc = cpu_ticks();
+ if (is_service_vm(vm)) {
+ calibrate_setup_timer();
+ } else {
+ vrtc_set_basetime(&vm->vrtc);
+ vm->vrtc.base_tsc = cpu_ticks();
+ }
}
--
2.25.1


[PATCH v5 6/8] hv: add time modification of vrtc

Zhao, Yuanyuan
 

VRTC for hv used to ignore writes to vRTC register.

This patch add time modification to vRTC.
Add base RTC time and TSC offset to get the current time. Convert
current time to vRTC register values (`struct rtcdev`). Then
modify a register, and calculate a new time. Get RTC offset by
substrcting new time from current time.
Thereafter, add RTC offset also when get current time. Then user
can get the modified time.

Signed-off-by: Yuanyuan Zhao <yuanyuan.zhao@...>
---
hypervisor/dm/vrtc.c | 14 +++++++++++++-
hypervisor/include/dm/vrtc.h | 2 ++
2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/hypervisor/dm/vrtc.c b/hypervisor/dm/vrtc.c
index 7295d7d58..4a404cb69 100644
--- a/hypervisor/dm/vrtc.c
+++ b/hypervisor/dm/vrtc.c
@@ -383,7 +383,7 @@ static time_t vrtc_get_current_time(struct acrn_vrtc *vrtc)

if (vrtc->base_rtctime > 0) {
offset = (cpu_ticks() - vrtc->base_tsc) / (get_tsc_khz() * 1000U);
- second = vrtc->base_rtctime + (time_t)offset;
+ second = vrtc->base_rtctime + vrtc->offset_rtctime + (time_t)offset;
}
return second;
}
@@ -527,6 +527,7 @@ static bool vrtc_read(struct acrn_vcpu *vcpu, uint16_t addr, __unused size_t wid
static bool vrtc_write(struct acrn_vcpu *vcpu, uint16_t addr, size_t width,
uint32_t value)
{
+ time_t current, after;
struct acrn_vrtc *vrtc = &vcpu->vm->vrtc;

if ((width == 1U) && (addr == CMOS_ADDR_PORT)) {
@@ -552,7 +553,18 @@ static bool vrtc_write(struct acrn_vcpu *vcpu, uint16_t addr, size_t width,
*((uint8_t *)&vrtc->rtcdev + vrtc->addr) = (uint8_t)(value & 0x7FU);
RTC_DEBUG("RTC alarm reg(%d) set to %#x (ignored)\n", vrtc->addr, value);
break;
+ case RTC_SEC:
+ /*
+ * High order bit of 'seconds' is readonly.
+ */
+ value &= 0x7f;
+ /* FALLTHRU */
default:
+ RTC_DEBUG("RTC offset %#x set to %#x\n", vrtc->addr, value);
+ *((uint8_t *)&vrtc->rtcdev + vrtc->addr) = (uint8_t)(value & 0xFFU);
+ current = vrtc_get_current_time(vrtc);
+ after = rtc_to_secs(vrtc);
+ vrtc->offset_rtctime += after - current;
break;
}
}
diff --git a/hypervisor/include/dm/vrtc.h b/hypervisor/include/dm/vrtc.h
index fff2559d2..b760ce445 100644
--- a/hypervisor/include/dm/vrtc.h
+++ b/hypervisor/include/dm/vrtc.h
@@ -34,6 +34,8 @@ struct acrn_vrtc {
uint32_t addr; /* RTC register to read or write */

time_t base_rtctime; /* Base time calulated from physical rtc register. */
+ time_t offset_rtctime; /* RTC offset against base time. */
+
uint64_t base_tsc; /* Base tsc value */

struct rtcdev rtcdev; /* RTC register */
--
2.25.1


[PATCH v5 5/8] hv: add Service VM write physical RTC register

Zhao, Yuanyuan
 

Service VM write physical RTC register.
Both RTC time modify and Configuration register is available.

Reviewed-by: Junjie Mao <junjie.mao@...>
Signed-off-by: Yuanyuan Zhao <yuanyuan.zhao@...>
---
hypervisor/dm/vrtc.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/hypervisor/dm/vrtc.c b/hypervisor/dm/vrtc.c
index 1d2416df2..7295d7d58 100644
--- a/hypervisor/dm/vrtc.c
+++ b/hypervisor/dm/vrtc.c
@@ -399,6 +399,12 @@ static uint8_t cmos_read(uint8_t addr)
return pio_read8(CMOS_DATA_PORT);
}

+static void cmos_write(uint8_t addr, uint8_t value)
+{
+ pio_write8(addr, CMOS_ADDR_PORT);
+ pio_write8(value, CMOS_DATA_PORT);
+}
+
static bool cmos_update_in_progress(void)
{
return (cmos_read(RTC_STATUSA) & RTCSA_TUP)?1:0;
@@ -422,6 +428,22 @@ static uint8_t cmos_get_reg_val(uint8_t addr)
return reg;
}

+static void cmos_set_reg_val(uint8_t addr, uint8_t value)
+{
+ int32_t tries = 2000;
+
+ spinlock_obtain(&cmos_lock);
+
+ /* Make sure an update isn't in progress */
+ while (cmos_update_in_progress() && (tries != 0)) {
+ tries -= 1;
+ }
+
+ cmos_write(addr, value);
+
+ spinlock_release(&cmos_lock);
+}
+
#define TRIGGER_ALARM (RTCIR_ALARM | RTCIR_INT)
#define RTC_DELTA 1 /* For RTC and system time may out of sync for no more than 1s */
static inline bool rtc_halted(struct acrn_vrtc *rtc)
@@ -510,7 +532,9 @@ static bool vrtc_write(struct acrn_vcpu *vcpu, uint16_t addr, size_t width,
if ((width == 1U) && (addr == CMOS_ADDR_PORT)) {
vrtc->addr = (uint8_t)(value & 0x7FU);
} else {
- if (!is_service_vm(vcpu->vm)) {
+ if (is_service_vm(vcpu->vm)) {
+ cmos_set_reg_val(vrtc->addr, (uint8_t)(value & 0xFFU));
+ } else {
switch (vrtc->addr) {
case RTC_STATUSA:
case RTC_INTR:
--
2.25.1


[PATCH v5 4/8] hv: add vRTC reg_b and reg_c support

Zhao, Yuanyuan
 

During setting RTC time, driver will halt RTC update. So support
bit 7 of reg_b. When it set to 0, time will be updated. And
when it's set to 1, rtc will keep the second it was set.

In the process of getting RTC time, driver sets alarm interrupt,
waits 1s, and get alarm interrupt flag. So support alarm interrupt
flag update. If alarm interrupt is enabled (bit 5, reg_b set to 1),
interrupt flag register will be set (bit 7 & bit 5 of reg_c) at
appropriate time.

Signed-off-by: Yuanyuan Zhao <yuanyuan.zhao@...>
---
hypervisor/dm/vrtc.c | 70 +++++++++++++++++++++++++++--
hypervisor/include/dm/mc146818rtc.h | 5 +++
2 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/hypervisor/dm/vrtc.c b/hypervisor/dm/vrtc.c
index 79ee2439d..1d2416df2 100644
--- a/hypervisor/dm/vrtc.c
+++ b/hypervisor/dm/vrtc.c
@@ -422,6 +422,41 @@ static uint8_t cmos_get_reg_val(uint8_t addr)
return reg;
}

+#define TRIGGER_ALARM (RTCIR_ALARM | RTCIR_INT)
+#define RTC_DELTA 1 /* For RTC and system time may out of sync for no more than 1s */
+static inline bool rtc_halted(struct acrn_vrtc *rtc)
+{
+ return ((rtc->rtcdev.reg_b & RTCSB_HALT) != 0U);
+}
+
+static uint8_t vrtc_get_reg_c(struct acrn_vrtc *vrtc)
+{
+ uint8_t ret = vrtc->rtcdev.reg_c;
+ struct rtcdev *rtc = &vrtc->rtcdev;
+ time_t current, alarm;
+
+ if ((rtc->reg_b & RTCSB_AINTR) != 0U) {
+ current = rtc->hour * 3600 + rtc->min * 60 + rtc->sec;
+ alarm = rtc->alarm_hour * 3600 + rtc->alarm_min * 60 + rtc->alarm_sec;
+
+ if (current >= alarm - RTC_DELTA && current <= alarm + RTC_DELTA) {
+ /*
+ * If alarm interrut is enabled and rtc time is in alarm time scale,
+ * set the interrupt flag as alarm triggered.
+ */
+ ret |= TRIGGER_ALARM;
+ }
+ }
+
+ vrtc->rtcdev.reg_c = 0;
+ return ret;
+}
+
+static void vrtc_set_reg_b(struct acrn_vrtc *vrtc, uint8_t newval)
+{
+ vrtc->rtcdev.reg_b = newval;
+}
+
/**
* @pre vcpu != NULL
* @pre vcpu->vm != NULL
@@ -435,7 +470,7 @@ static bool vrtc_read(struct acrn_vcpu *vcpu, uint16_t addr, __unused size_t wid
struct acrn_vm *vm = vcpu->vm;
bool ret = true;

- offset = vm->vrtc.addr;
+ offset = vrtc->addr;

if (addr == CMOS_ADDR_PORT) {
pio_req->value = offset;
@@ -447,7 +482,11 @@ static bool vrtc_read(struct acrn_vcpu *vcpu, uint16_t addr, __unused size_t wid
current = vrtc_get_current_time(vrtc);
secs_to_rtc(current, vrtc);

- pio_req->value = *((uint8_t *)&vm->vrtc.rtcdev + offset);
+ if(offset == 0xCU) {
+ pio_req->value = vrtc_get_reg_c(vrtc);
+ } else {
+ pio_req->value = *((uint8_t *)&vrtc->rtcdev + offset);
+ }
RTC_DEBUG("read 0x%x, 0x%x", offset, pio_req->value);
} else {
RTC_ERR("vrtc read invalid addr 0x%x", offset);
@@ -466,8 +505,33 @@ static bool vrtc_read(struct acrn_vcpu *vcpu, uint16_t addr, __unused size_t wid
static bool vrtc_write(struct acrn_vcpu *vcpu, uint16_t addr, size_t width,
uint32_t value)
{
+ struct acrn_vrtc *vrtc = &vcpu->vm->vrtc;
+
if ((width == 1U) && (addr == CMOS_ADDR_PORT)) {
- vcpu->vm->vrtc.addr = (uint8_t)value & 0x7FU;
+ vrtc->addr = (uint8_t)(value & 0x7FU);
+ } else {
+ if (!is_service_vm(vcpu->vm)) {
+ switch (vrtc->addr) {
+ case RTC_STATUSA:
+ case RTC_INTR:
+ case RTC_STATUSD:
+ RTC_DEBUG("RTC reg_%x set to %#x (ignored)\n", vrtc->addr, value);
+ break;
+ case RTC_STATUSB:
+ vrtc_set_reg_b(vrtc, value);
+ RTC_DEBUG("RTC reg_b set to %#x\n", value);
+ break;
+ case RTC_SECALRM:
+ case RTC_MINALRM:
+ /* FALLTHRU */
+ case RTC_HRSALRM:
+ *((uint8_t *)&vrtc->rtcdev + vrtc->addr) = (uint8_t)(value & 0x7FU);
+ RTC_DEBUG("RTC alarm reg(%d) set to %#x (ignored)\n", vrtc->addr, value);
+ break;
+ default:
+ break;
+ }
+ }
}

return true;
diff --git a/hypervisor/include/dm/mc146818rtc.h b/hypervisor/include/dm/mc146818rtc.h
index b23f90d34..1681cc960 100644
--- a/hypervisor/include/dm/mc146818rtc.h
+++ b/hypervisor/include/dm/mc146818rtc.h
@@ -60,8 +60,13 @@
#define RTC_STATUSB 0x0b /* status register B */
#define RTCSB_24HR 0x02 /* 0 = 12 hours, 1 = 24 hours */
#define RTCSB_BCD 0x04 /* 0 = BCD, 1 = Binary coded time */
+#define RTCSB_AINTR 0x20 /* 1 = enable alarm interrupt */
+#define RTCSB_HALT 0x80 /* stop clock updates */

#define RTC_INTR 0x0c /* status register C (R) interrupt source */
+#define RTCIR_ALARM 0x20 /* alarm intr */
+#define RTCIR_INT 0x80 /* interrupt output signal */
+
#define RTC_STATUSD 0x0d /* status register D (R) Lost Power */

#endif /* _MC146818_RTC_H_ */
--
2.25.1


[PATCH v5 3/8] hv: support 12 hour mode

Zhao, Yuanyuan
 

Add support for hour format.
Bit 1 of register B indicates the hour byte format. When it's 0,
twelve-hour mode is selected. Then the seventh bit of hour register
presents AM as 0 and PM as 1.

Signed-off-by: Yuanyuan Zhao <yuanyuan.zhao@...>
---
hypervisor/dm/vrtc.c | 76 +++++++++++++++++++++++++++--
hypervisor/include/dm/mc146818rtc.h | 1 +
2 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/hypervisor/dm/vrtc.c b/hypervisor/dm/vrtc.c
index d65c27cbf..79ee2439d 100644
--- a/hypervisor/dm/vrtc.c
+++ b/hypervisor/dm/vrtc.c
@@ -239,12 +239,12 @@ static time_t rtc_to_secs(const struct acrn_vrtc *vrtc)
struct clktime ct;
time_t second = VRTC_BROKEN_TIME;
const struct rtcdev *rtc= &vrtc->rtcdev;
- int32_t err = 0;
+ int32_t err = 0, hour = 0, pm = 0;
uint32_t century = 0, year = 0;

do {
if (rtcget(rtc, rtc->sec, &ct.sec) < 0 || rtcget(rtc, rtc->min, &ct.min) < 0 ||
- rtcget(rtc, rtc->hour, &ct.hour) < 0 || rtcget(rtc, rtc->day_of_month, &ct.day) < 0 ||
+ rtcget(rtc, rtc->day_of_month, &ct.day) < 0 ||
rtcget(rtc, rtc->month, &ct.mon) < 0 || rtcget(rtc, rtc->year, &year) < 0 ||
rtcget(rtc, rtc->century, &century) < 0) {
RTC_ERR("Invalid RTC sec %#x hour %#x day %#x mon %#x year %#x century %#x\n",
@@ -254,6 +254,47 @@ static time_t rtc_to_secs(const struct acrn_vrtc *vrtc)
break;
}

+ /*
+ * If 12 hour format is inuse, translate it to 24 hour format here.
+ */
+ pm = 0;
+ hour = rtc->hour;
+ if ((rtc->reg_b & RTCSB_24HR) == 0) {
+ if (hour & 0x80U) {
+ hour &= ~0x80U;
+ pm = 1;
+ }
+ }
+ err = rtcget(rtc, hour, &ct.hour);
+ if (err) {
+ RTC_ERR("Invalid RTC hour %#x/%d\n", rtc->hour, ct.hour);
+ break;
+ }
+ if ((rtc->reg_b & RTCSB_24HR) == 0) {
+ if (ct.hour >= 1 && ct.hour <= 12) {
+ /*
+ * Convert from 12-hour format to internal 24-hour
+ * representation as follows:
+ *
+ * 12-hour format ct.hour
+ * 12 AM 0
+ * 1 - 11 AM 1 - 11
+ * 12 PM 12
+ * 1 - 11 PM 13 - 23
+ */
+ if (ct.hour == 12) {
+ ct.hour = 0;
+ }
+ if (pm) {
+ ct.hour += 12;
+ }
+ } else {
+ RTC_ERR("Invalid RTC 12-hour format %#x/%d\n",
+ rtc->hour, ct.hour);
+ break;
+ }
+ }
+
/*
* Ignore 'rtc->dow' because some guests like Linux don't bother
* setting it at all while others like OpenBSD/i386 set it incorrectly.
@@ -289,12 +330,41 @@ static void secs_to_rtc(time_t rtctime, struct acrn_vrtc *vrtc)
{
struct clktime ct;
struct rtcdev *rtc;
+ int32_t hour;

if (rtctime > 0 && clk_ts_to_ct(rtctime, &ct) == 0) {
rtc = &vrtc->rtcdev;
rtc->sec = rtcset(rtc, ct.sec);
rtc->min = rtcset(rtc, ct.min);
- rtc->hour = rtcset(rtc, ct.hour);
+
+ if (rtc->reg_b & RTCSB_24HR) {
+ hour = ct.hour;
+ } else {
+ /*
+ * Convert to the 12-hour format.
+ */
+ switch (ct.hour) {
+ case 0: /* 12 AM */
+ case 12: /* 12 PM */
+ hour = 12;
+ break;
+ default:
+ /*
+ * The remaining 'ct.hour' values are interpreted as:
+ * [1 - 11] -> 1 - 11 AM
+ * [13 - 23] -> 1 - 11 PM
+ */
+ hour = ct.hour % 12;
+ break;
+ }
+ }
+
+ rtc->hour = rtcset(rtc, hour);
+
+ if ((rtc->reg_b & RTCSB_24HR) == 0 && ct.hour >= 12) {
+ rtc->hour |= 0x80; /* set MSB to indicate PM */
+ }
+
rtc->day_of_week = rtcset(rtc, ct.dow + 1);
rtc->day_of_month = rtcset(rtc, ct.day);
rtc->month = rtcset(rtc, ct.mon);
diff --git a/hypervisor/include/dm/mc146818rtc.h b/hypervisor/include/dm/mc146818rtc.h
index 4baf9de69..b23f90d34 100644
--- a/hypervisor/include/dm/mc146818rtc.h
+++ b/hypervisor/include/dm/mc146818rtc.h
@@ -58,6 +58,7 @@
#define RTCSA_TUP 0x80 /* time update, don't look now */

#define RTC_STATUSB 0x0b /* status register B */
+#define RTCSB_24HR 0x02 /* 0 = 12 hours, 1 = 24 hours */
#define RTCSB_BCD 0x04 /* 0 = BCD, 1 = Binary coded time */

#define RTC_INTR 0x0c /* status register C (R) interrupt source */
--
2.25.1


[PATCH v5 2/8] hv: support vrtc BCD data mode

Zhao, Yuanyuan
 

Add judging of the vRTC data mode. If BCD data mode is inuse,
make conversion of data.

Reviewed-by: Junjie Mao <junjie.mao@...>
Signed-off-by: Yuanyuan Zhao <yuanyuan.zhao@...>
---
hypervisor/dm/vrtc.c | 79 ++++++++++++++++++++++++-----
hypervisor/include/dm/mc146818rtc.h | 2 +
2 files changed, 67 insertions(+), 14 deletions(-)

diff --git a/hypervisor/dm/vrtc.c b/hypervisor/dm/vrtc.c
index 14c836028..d65c27cbf 100644
--- a/hypervisor/dm/vrtc.c
+++ b/hypervisor/dm/vrtc.c
@@ -104,6 +104,51 @@ static inline uint32_t day_of_week(uint32_t days)
return ((days) + 4U) % 7U;
}

+uint8_t const bin2bcd_data[] = {
+ 0x00U, 0x01U, 0x02U, 0x03U, 0x04U, 0x05U, 0x06U, 0x07U, 0x08U, 0x09U,
+ 0x10U, 0x11U, 0x12U, 0x13U, 0x14U, 0x15U, 0x16U, 0x17U, 0x18U, 0x19U,
+ 0x20U, 0x21U, 0x22U, 0x23U, 0x24U, 0x25U, 0x26U, 0x27U, 0x28U, 0x29U,
+ 0x30U, 0x31U, 0x32U, 0x33U, 0x34U, 0x35U, 0x36U, 0x37U, 0x38U, 0x39U,
+ 0x40U, 0x41U, 0x42U, 0x43U, 0x44U, 0x45U, 0x46U, 0x47U, 0x48U, 0x49U,
+ 0x50U, 0x51U, 0x52U, 0x53U, 0x54U, 0x55U, 0x56U, 0x57U, 0x58U, 0x59U,
+ 0x60U, 0x61U, 0x62U, 0x63U, 0x64U, 0x65U, 0x66U, 0x67U, 0x68U, 0x69U,
+ 0x70U, 0x71U, 0x72U, 0x73U, 0x74U, 0x75U, 0x76U, 0x77U, 0x78U, 0x79U,
+ 0x80U, 0x81U, 0x82U, 0x83U, 0x84U, 0x85U, 0x86U, 0x87U, 0x88U, 0x89U,
+ 0x90U, 0x91U, 0x92U, 0x93U, 0x94U, 0x95U, 0x96U, 0x97U, 0x98U, 0x99U
+};
+
+/*
+ * @pre val < 100
+ */
+static inline uint8_t rtcset(struct rtcdev *rtc, uint32_t val)
+{
+ return ((rtc->reg_b & RTCSB_BCD) ? val : bin2bcd_data[val]);
+}
+
+/*
+ * Get rtc time register binary value.
+ * If BCD data mode is enabled, translate BCD to binary.
+ */
+static int32_t rtcget(const struct rtcdev *rtc, int32_t val, uint32_t *retval)
+{
+ uint8_t upper, lower;
+ int32_t errno = 0;
+
+ if (rtc->reg_b & RTCSB_BCD) {
+ *retval = val;
+ } else {
+ lower = val & 0xfU;
+ upper = (val >> 4) & 0xfU;
+
+ if (lower > 9U || upper > 9U) {
+ errno = -EINVAL;
+ } else {
+ *retval = upper * 10U + lower;
+ }
+ }
+ return errno;
+}
+
/*
* Translate clktime (such as year, month, day) to time_t.
*/
@@ -195,13 +240,19 @@ static time_t rtc_to_secs(const struct acrn_vrtc *vrtc)
time_t second = VRTC_BROKEN_TIME;
const struct rtcdev *rtc= &vrtc->rtcdev;
int32_t err = 0;
+ uint32_t century = 0, year = 0;

do {
- ct.sec = rtc->sec;
- ct.min = rtc->min;
- ct.hour = rtc->hour;
- ct.day = rtc->day_of_month;
- ct.mon = rtc->month;
+ if (rtcget(rtc, rtc->sec, &ct.sec) < 0 || rtcget(rtc, rtc->min, &ct.min) < 0 ||
+ rtcget(rtc, rtc->hour, &ct.hour) < 0 || rtcget(rtc, rtc->day_of_month, &ct.day) < 0 ||
+ rtcget(rtc, rtc->month, &ct.mon) < 0 || rtcget(rtc, rtc->year, &year) < 0 ||
+ rtcget(rtc, rtc->century, &century) < 0) {
+ RTC_ERR("Invalid RTC sec %#x hour %#x day %#x mon %#x year %#x century %#x\n",
+ rtc->sec, rtc->min, rtc->day_of_month, rtc->month,
+ rtc->year, rtc->century);
+ err = -1;
+ break;
+ }

/*
* Ignore 'rtc->dow' because some guests like Linux don't bother
@@ -211,7 +262,7 @@ static time_t rtc_to_secs(const struct acrn_vrtc *vrtc)
*/
ct.dow = -1;

- ct.year = rtc->century * 100 + rtc->year;
+ ct.year = century * 100 + year;
if (ct.year < POSIX_BASE_YEAR) {
RTC_ERR("Invalid RTC century %#x/%d\n", rtc->century,
ct.year);
@@ -241,14 +292,14 @@ static void secs_to_rtc(time_t rtctime, struct acrn_vrtc *vrtc)

if (rtctime > 0 && clk_ts_to_ct(rtctime, &ct) == 0) {
rtc = &vrtc->rtcdev;
- rtc->sec = ct.sec;
- rtc->min = ct.min;
- rtc->hour = ct.hour;
- rtc->day_of_week = ct.dow + 1;
- rtc->day_of_month = ct.day;
- rtc->month = ct.mon;
- rtc->year = ct.year % 100;
- rtc->century = ct.year / 100;
+ rtc->sec = rtcset(rtc, ct.sec);
+ rtc->min = rtcset(rtc, ct.min);
+ rtc->hour = rtcset(rtc, ct.hour);
+ rtc->day_of_week = rtcset(rtc, ct.dow + 1);
+ rtc->day_of_month = rtcset(rtc, ct.day);
+ rtc->month = rtcset(rtc, ct.mon);
+ rtc->year = rtcset(rtc, ct.year % 100);
+ rtc->century = rtcset(rtc, ct.year / 100);
}
}

diff --git a/hypervisor/include/dm/mc146818rtc.h b/hypervisor/include/dm/mc146818rtc.h
index 023bbe599..4baf9de69 100644
--- a/hypervisor/include/dm/mc146818rtc.h
+++ b/hypervisor/include/dm/mc146818rtc.h
@@ -58,6 +58,8 @@
#define RTCSA_TUP 0x80 /* time update, don't look now */

#define RTC_STATUSB 0x0b /* status register B */
+#define RTCSB_BCD 0x04 /* 0 = BCD, 1 = Binary coded time */
+
#define RTC_INTR 0x0c /* status register C (R) interrupt source */
#define RTC_STATUSD 0x0d /* status register D (R) Lost Power */

--
2.25.1


[PATCH v5 1/8] hv: add `rtcdev` emulate vrtc

Zhao, Yuanyuan
 

Current code would read physical RTC register and return it directly to guest.

This patch would read a base physical RTC time and a base physical TSC time
at initialize stage. Then when guest tries to read vRTC time, ACRN HV would
read the real TSC time and use the TSC offset to calculate the real RTC time.

This patch only support BIN data mode and 24 hour mode.
BCD data mode and 12 hour mode will add in other patch.
The accuracy of clock provided by this patch is limited by TSC, and will
be improved in a following patch also.

Reviewed-by: Junjie Mao <junjie.mao@...>
Signed-off-by: Yuanyuan Zhao <yuanyuan.zhao@...>
---
hypervisor/dm/vrtc.c | 321 ++++++++++++++++++++-
hypervisor/include/arch/x86/asm/guest/vm.h | 3 +-
hypervisor/include/dm/mc146818rtc.h | 64 ++++
hypervisor/include/dm/vrtc.h | 42 +++
4 files changed, 418 insertions(+), 12 deletions(-)

diff --git a/hypervisor/dm/vrtc.c b/hypervisor/dm/vrtc.c
index 23fb978c2..14c836028 100644
--- a/hypervisor/dm/vrtc.c
+++ b/hypervisor/dm/vrtc.c
@@ -1,18 +1,275 @@
/*
- * Copyright (C) 2018 Intel Corporation.
+ * Copyright (c) 2014, Neel Natu (neel@...)
+ * Copyright (c) 2022 Intel Corporation
+ * All rights reserved.
*
- * SPDX-License-Identifier: BSD-3-Clause
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice unmodified, this list of conditions, and the following
+ * disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

#include <asm/guest/vm.h>
#include <asm/io.h>
+#include <asm/tsc.h>
+#include <vrtc.h>
+#include <logmsg.h>
+
+#include "mc146818rtc.h"
+
+/* #define DEBUG_RTC */
+#ifdef DEBUG_RTC
+# define RTC_DEBUG pr_info
+#else
+# define RTC_DEBUG(format, ...) do { } while (false)
+#endif
+# define RTC_ERR pr_err
+
+struct clktime {
+ uint32_t year; /* year (4 digit year) */
+ uint32_t mon; /* month (1 - 12) */
+ uint32_t day; /* day (1 - 31) */
+ uint32_t hour; /* hour (0 - 23) */
+ uint32_t min; /* minute (0 - 59) */
+ uint32_t sec; /* second (0 - 59) */
+ uint32_t dow; /* day of week (0 - 6; 0 = Sunday) */
+};
+
+#define POSIX_BASE_YEAR 1970
+#define SECDAY (24 * 60 * 60)
+#define SECYR (SECDAY * 365)
+#define VRTC_BROKEN_TIME ((time_t)-1)
+
+#define FEBRUARY 2U
+
+static const uint32_t month_days[12] = {
+ 31U, 28U, 31U, 30U, 31U, 30U, 31U, 31U, 30U, 31U, 30U, 31U
+};
+
+/*
+ * This inline avoids some unnecessary modulo operations
+ * as compared with the usual macro:
+ * ( ((year % 4) == 0 &&
+ * (year % 100) != 0) ||
+ * ((year % 400) == 0) )
+ * It is otherwise equivalent.
+ */
+static inline uint32_t leapyear(uint32_t year)
+{
+ uint32_t rv = 0U;
+
+ if ((year & 3U) == 0) {
+ rv = 1U;
+ if ((year % 100U) == 0) {
+ rv = 0U;
+ if ((year % 400U) == 0) {
+ rv = 1U;
+ }
+ }
+ }
+ return rv;
+}
+
+static inline uint32_t days_in_year(uint32_t year)
+{
+ return leapyear(year) ? 366U : 365U;
+}
+
+static inline uint32_t days_in_month(uint32_t year, uint32_t month)
+{
+ return month_days[(month) - 1U] + (month == FEBRUARY ? leapyear(year) : 0U);
+}
+
+/*
+ * Day of week. Days are counted from 1/1/1970, which was a Thursday.
+ */
+static inline uint32_t day_of_week(uint32_t days)
+{
+ return ((days) + 4U) % 7U;
+}
+
+/*
+ * Translate clktime (such as year, month, day) to time_t.
+ */
+static int32_t clk_ct_to_ts(struct clktime *ct, time_t *sec)
+{
+ uint32_t i, year, days;
+ int32_t err = 0;
+
+ year = ct->year;
+
+ /* Sanity checks. */
+ if (ct->mon < 1U || ct->mon > 12U || ct->day < 1U ||
+ ct->day > days_in_month(year, ct->mon) ||
+ ct->hour > 23U || ct->min > 59U || ct->sec > 59U ||
+ year < POSIX_BASE_YEAR || year > 2037U) {
+ /* time_t overflow */
+ err = -EINVAL;
+ } else {
+ /*
+ * Compute days since start of time
+ * First from years, then from months.
+ */
+ days = 0U;
+ for (i = POSIX_BASE_YEAR; i < year; i++) {
+ days += days_in_year(i);
+ }
+
+ /* Months */
+ for (i = 1; i < ct->mon; i++) {
+ days += days_in_month(year, i);
+ }
+ days += (ct->day - 1);
+
+ *sec = (((time_t)days * 24 + ct->hour) * 60 + ct->min) * 60 + ct->sec;
+ }
+ return err;
+}
+
+/*
+ * Translate time_t to clktime (such as year, month, day)
+ */
+static int32_t clk_ts_to_ct(time_t secs, struct clktime *ct)
+{
+ uint32_t i, year, days;
+ time_t rsec; /* remainder seconds */
+ int32_t err = 0;
+
+ days = secs / SECDAY;
+ rsec = secs % SECDAY;
+
+ ct->dow = day_of_week(days);
+
+ /* Substract out whole years, counting them in i. */
+ for (year = POSIX_BASE_YEAR; days >= days_in_year(year); year++) {
+ days -= days_in_year(year);
+ }
+ ct->year = year;
+
+ /* Substract out whole months, counting them in i. */
+ for (i = 1; days >= days_in_month(year, i); i++) {
+ days -= days_in_month(year, i);
+ }
+ ct->mon = i;
+
+ /* Days are what is left over (+1) from all that. */
+ ct->day = days + 1;
+
+ /* Hours, minutes, seconds are easy */
+ ct->hour = rsec / 3600U;
+ rsec = rsec % 3600U;
+ ct->min = rsec / 60U;
+ rsec = rsec % 60U;
+ ct->sec = rsec;
+
+ /* time_t is defined as int32_t, so year should not be more than 2037. */
+ if (ct->mon > 12U || ct->year > 2037 || ct->day > days_in_month(ct->year, ct->mon)) {
+ RTC_ERR("Invalid vRTC param mon %d, year %d, day %d\n", ct->mon, ct->year, ct->day);
+ err = -EINVAL;
+ }
+ return err;
+}
+
+/*
+ * Calculate second value from rtcdev register info which save in vrtc.
+ */
+static time_t rtc_to_secs(const struct acrn_vrtc *vrtc)
+{
+ struct clktime ct;
+ time_t second = VRTC_BROKEN_TIME;
+ const struct rtcdev *rtc= &vrtc->rtcdev;
+ int32_t err = 0;
+
+ do {
+ ct.sec = rtc->sec;
+ ct.min = rtc->min;
+ ct.hour = rtc->hour;
+ ct.day = rtc->day_of_month;
+ ct.mon = rtc->month;
+
+ /*
+ * Ignore 'rtc->dow' because some guests like Linux don't bother
+ * setting it at all while others like OpenBSD/i386 set it incorrectly.
+ *
+ * clock_ct_to_ts() does not depend on 'ct.dow' anyways so ignore it.
+ */
+ ct.dow = -1;
+
+ ct.year = rtc->century * 100 + rtc->year;
+ if (ct.year < POSIX_BASE_YEAR) {
+ RTC_ERR("Invalid RTC century %#x/%d\n", rtc->century,
+ ct.year);
+ break;
+ }
+
+ err = clk_ct_to_ts(&ct, &second);
+ if (err) {
+ RTC_ERR("Invalid RTC clocktime.date %04d-%02d-%02d\n",
+ ct.year, ct.mon, ct.day);
+ RTC_ERR("Invalid RTC clocktime.time %02d:%02d:%02d\n",
+ ct.hour, ct.min, ct.sec);
+ break;
+ }
+ } while (false);
+
+ return second;
+}
+
+/*
+ * Translate second value to rtcdev register info and save it in vrtc.
+ */
+static void secs_to_rtc(time_t rtctime, struct acrn_vrtc *vrtc)
+{
+ struct clktime ct;
+ struct rtcdev *rtc;
+
+ if (rtctime > 0 && clk_ts_to_ct(rtctime, &ct) == 0) {
+ rtc = &vrtc->rtcdev;
+ rtc->sec = ct.sec;
+ rtc->min = ct.min;
+ rtc->hour = ct.hour;
+ rtc->day_of_week = ct.dow + 1;
+ rtc->day_of_month = ct.day;
+ rtc->month = ct.mon;
+ rtc->year = ct.year % 100;
+ rtc->century = ct.year / 100;
+ }
+}
+
+/*
+ * If the base_rtctime is valid, calculate current time by add tsc offset and offset_rtctime.
+ */
+static time_t vrtc_get_current_time(struct acrn_vrtc *vrtc)
+{
+ uint64_t offset;
+ time_t second = VRTC_BROKEN_TIME;
+
+ if (vrtc->base_rtctime > 0) {
+ offset = (cpu_ticks() - vrtc->base_tsc) / (get_tsc_khz() * 1000U);
+ second = vrtc->base_rtctime + (time_t)offset;
+ }
+ return second;
+}

#define CMOS_ADDR_PORT 0x70U
#define CMOS_DATA_PORT 0x71U

-#define RTC_STATUSA 0x0AU /* status register A */
-#define RTCSA_TUP 0x80U /* time update, don't look now */
-
static spinlock_t cmos_lock = { .head = 0U, .tail = 0U };

static uint8_t cmos_read(uint8_t addr)
@@ -51,18 +308,34 @@ static uint8_t cmos_get_reg_val(uint8_t addr)
static bool vrtc_read(struct acrn_vcpu *vcpu, uint16_t addr, __unused size_t width)
{
uint8_t offset;
+ time_t current;
+ struct acrn_vrtc *vrtc = &vcpu->vm->vrtc;
struct acrn_pio_request *pio_req = &vcpu->req.reqs.pio_request;
struct acrn_vm *vm = vcpu->vm;
+ bool ret = true;

- offset = vm->vrtc_offset;
+ offset = vm->vrtc.addr;

if (addr == CMOS_ADDR_PORT) {
- pio_req->value = vm->vrtc_offset;
+ pio_req->value = offset;
} else {
- pio_req->value = cmos_get_reg_val(offset);
+ if (is_service_vm(vm)) {
+ pio_req->value = cmos_get_reg_val(offset);
+ } else {
+ if (offset <= RTC_CENTURY) {
+ current = vrtc_get_current_time(vrtc);
+ secs_to_rtc(current, vrtc);
+
+ pio_req->value = *((uint8_t *)&vm->vrtc.rtcdev + offset);
+ RTC_DEBUG("read 0x%x, 0x%x", offset, pio_req->value);
+ } else {
+ RTC_ERR("vrtc read invalid addr 0x%x", offset);
+ ret = false;
+ }
+ }
}

- return true;
+ return ret;
}

/**
@@ -73,19 +346,45 @@ static bool vrtc_write(struct acrn_vcpu *vcpu, uint16_t addr, size_t width,
uint32_t value)
{
if ((width == 1U) && (addr == CMOS_ADDR_PORT)) {
- vcpu->vm->vrtc_offset = (uint8_t)value & 0x7FU;
+ vcpu->vm->vrtc.addr = (uint8_t)value & 0x7FU;
}

return true;
}

+static void vrtc_set_basetime(struct acrn_vrtc *vrtc)
+{
+ struct rtcdev *vrtcdev = &vrtc->rtcdev;
+
+ /*
+ * Read base time from physical rtc.
+ */
+ vrtcdev->sec = cmos_get_reg_val(RTC_SEC);
+ vrtcdev->min = cmos_get_reg_val(RTC_MIN);
+ vrtcdev->hour = cmos_get_reg_val(RTC_HRS);
+ vrtcdev->day_of_month = cmos_get_reg_val(RTC_DAY);
+ vrtcdev->month = cmos_get_reg_val(RTC_MONTH);
+ vrtcdev->year = cmos_get_reg_val(RTC_YEAR);
+ vrtcdev->century = cmos_get_reg_val(RTC_CENTURY);
+ vrtcdev->reg_a = cmos_get_reg_val(RTC_STATUSA) & (~RTCSA_TUP);
+ vrtcdev->reg_b = cmos_get_reg_val(RTC_STATUSB);
+ vrtcdev->reg_c = cmos_get_reg_val(RTC_INTR);
+ vrtcdev->reg_d = cmos_get_reg_val(RTC_STATUSD);
+
+ vrtc->base_rtctime = rtc_to_secs(vrtc);
+}
+
void vrtc_init(struct acrn_vm *vm)
{
struct vm_io_range range = {
.base = CMOS_ADDR_PORT, .len = 2U};

/* Initializing the CMOS RAM offset to 0U */
- vm->vrtc_offset = 0U;
+ vm->vrtc.addr = 0U;

+ vm->vrtc.vm = vm;
register_pio_emulation_handler(vm, RTC_PIO_IDX, &range, vrtc_read, vrtc_write);
+
+ vrtc_set_basetime(&vm->vrtc);
+ vm->vrtc.base_tsc = cpu_ticks();
}
diff --git a/hypervisor/include/arch/x86/asm/guest/vm.h b/hypervisor/include/arch/x86/asm/guest/vm.h
index 589d0a913..c05b9b212 100644
--- a/hypervisor/include/arch/x86/asm/guest/vm.h
+++ b/hypervisor/include/arch/x86/asm/guest/vm.h
@@ -21,6 +21,7 @@
#include <vpic.h>
#include <asm/guest/vmx_io.h>
#include <vuart.h>
+#include <vrtc.h>
#include <asm/guest/trusty.h>
#include <asm/guest/vcpuid.h>
#include <vpci.h>
@@ -169,7 +170,7 @@ struct acrn_vm {
uint32_t vcpuid_entry_nr, vcpuid_level, vcpuid_xlevel;
struct vcpuid_entry vcpuid_entries[MAX_VM_VCPUID_ENTRIES];
struct acrn_vpci vpci;
- uint8_t vrtc_offset;
+ struct acrn_vrtc vrtc;

uint64_t intr_inject_delay_delta; /* delay of intr injection */
} __aligned(PAGE_SIZE);
diff --git a/hypervisor/include/dm/mc146818rtc.h b/hypervisor/include/dm/mc146818rtc.h
new file mode 100644
index 000000000..023bbe599
--- /dev/null
+++ b/hypervisor/include/dm/mc146818rtc.h
@@ -0,0 +1,64 @@
+/*-
+ * Copyright (c) 1990 The Regents of the University of California.
+ * All rights reserved.
+ *
+ * Copyright (C) 2022 Intel Corporation.
+ *
+ * This code is derived from software contributed to Berkeley by
+ * William Jolitz.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the University nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * from: @(#)rtc.h 7.1 (Berkeley) 5/12/91
+ * $FreeBSD$
+ */
+
+#ifndef _MC146818_RTC_H_
+#define _MC146818_RTC_H_
+
+/*
+ * MC146818 RTC Register locations
+ */
+
+#define RTC_SEC 0x00 /* seconds */
+#define RTC_SECALRM 0x01 /* seconds alarm */
+#define RTC_MIN 0x02 /* minutes */
+#define RTC_MINALRM 0x03 /* minutes alarm */
+#define RTC_HRS 0x04 /* hours */
+#define RTC_HRSALRM 0x05 /* hours alarm */
+#define RTC_WDAY 0x06 /* week day */
+#define RTC_DAY 0x07 /* day of month */
+#define RTC_MONTH 0x08 /* month of year */
+#define RTC_YEAR 0x09 /* year in century*/
+#define RTC_CENTURY 0x32 /* century */
+
+#define RTC_STATUSA 0x0a /* status register A */
+#define RTCSA_TUP 0x80 /* time update, don't look now */
+
+#define RTC_STATUSB 0x0b /* status register B */
+#define RTC_INTR 0x0c /* status register C (R) interrupt source */
+#define RTC_STATUSD 0x0d /* status register D (R) Lost Power */
+
+#endif /* _MC146818_RTC_H_ */
diff --git a/hypervisor/include/dm/vrtc.h b/hypervisor/include/dm/vrtc.h
new file mode 100644
index 000000000..fff2559d2
--- /dev/null
+++ b/hypervisor/include/dm/vrtc.h
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2022 Intel Corporation.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#ifndef VRTC_H
+#define VRTC_H
+
+typedef int32_t time_t;
+
+/* Register layout of the RTC */
+struct rtcdev {
+ uint8_t sec;
+ uint8_t alarm_sec;
+ uint8_t min;
+ uint8_t alarm_min;
+ uint8_t hour;
+ uint8_t alarm_hour;
+ uint8_t day_of_week;
+ uint8_t day_of_month;
+ uint8_t month;
+ uint8_t year;
+ uint8_t reg_a;
+ uint8_t reg_b;
+ uint8_t reg_c;
+ uint8_t reg_d;
+ uint8_t res[36];
+ uint8_t century;
+};
+
+struct acrn_vrtc {
+ struct acrn_vm *vm;
+ uint32_t addr; /* RTC register to read or write */
+
+ time_t base_rtctime; /* Base time calulated from physical rtc register. */
+ uint64_t base_tsc; /* Base tsc value */
+
+ struct rtcdev rtcdev; /* RTC register */
+};
+
+#endif /* VRTC_H */
--
2.25.1


[PATCH v5 0/8] add vRTC write emulate

Zhao, Yuanyuan
 

Now ACRN would emulate vRTC for non-RT pre-launched VM, RTVM and
Service VM.
For vRTC read, ACRN would only return the the physical RTC register value;
for vRTC write, ACRN would ignore it.

New implement is:
1. Service OS read and write physical RTC.
2. The initial vRTC of each VM will is same with physical RTC, i.e. the RTC of service VM.
3. Guest VM can re-program its RTC (thu NTP etc.), but the update of guest RTC will be
emulated by Hypervisor through 'offset_rtctime'.
4. Service VM will update physical RTC. So, if the system shuts down, hypervisor can rely
on #2 for each VM to get right initial RTC (sync with real NTP), without saving the “offset_rtctime”.
5. A calibration of vrtc will trigger by timer every 3 hours to keep
precision.
6. When write the physical RTC, will reset the 'offset_rtctime' of each
guest VM.

v4->v5
1. [1/8] 'vrtc_set_basetime' change byte array to 'struct rtcdev' ptr.
2. [3/8] Remove space.
3. [4/8][5/8] Commit message.
4. [6/8] 'vrtc_read' switch case '0' to 'RTC_SEC'.
5. [7/8] 'calibrate_lock' --> 'vrtc_rebase_lock'.

v3->v4:
1. Add a calibrate_lock.
2. Split reg_b and reg_c patch.
3. Other commits in v3.

v2->v3:
1. Remove Servcie VM pass through, add read & write for it in HV.
2. Add periodical calibation of vrtc.
3. When set RTC time, reset offset of vrtc.

v1->v2:

1. Change name of 'mc146818rtc.h'.
2. Add cover letter.
3. Use unsigned int in time transform.
4. Split patches in a new way.

Yuanyuan Zhao (8):
hv: add `rtcdev` emulate vrtc
hv: support vrtc BCD data mode
hv: support 12 hour mode
hv: add vRTC reg_b and reg_c support
hv: add Service VM write physical RTC register
hv: add time modification of vrtc
hv: vrtc calibrate function
hv: calibrate vrtc when modify physical time

hypervisor/dm/vrtc.c | 635 ++++++++++++++++++++-
hypervisor/include/arch/x86/asm/guest/vm.h | 3 +-
hypervisor/include/dm/mc146818rtc.h | 72 +++
hypervisor/include/dm/vrtc.h | 44 ++
4 files changed, 742 insertions(+), 12 deletions(-)
create mode 100644 hypervisor/include/dm/mc146818rtc.h
create mode 100644 hypervisor/include/dm/vrtc.h

--
2.25.1


Re: [PATCH] hv: fix the issue of "right shift count"

chenli.wei
 

On 5/7/2022 11:17 AM, Li, Fei1 wrote:
On Sat, May 07, 2022 at 06:09:02AM +0800, Dong, Eddie wrote:

-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of chenli.wei
Sent: Friday, May 6, 2022 1:23 AM
To: Li, Fei1 <fei1.li@...>; acrn-dev@...
Cc: Wei, Chenli <chenli.wei@...>
Subject: [acrn-dev] [PATCH] hv: fix the issue of "right shift count"

The current code have use powerof2_roundup to get the 2^n, there was an
issue if used to get the 2^n of a macro, that's because macros have no data
type.
Didn't quite understand.


BTW, the current logic of powerof2_roundup() is very ugly, and I am wondering why we need the following logic:

#if (CONFIG_MAX_PT_IRQ_ENTRIES <= 256)
#define MAX_IR_ENTRIES 256
#else
#define MAX_IR_ENTRIES powerof2_roundup(CONFIG_MAX_PT_IRQ_ENTRIES)
#endif
Interrupt Remapping Table Address Register (IRTA_REG)

63:12 : Interrupt Remapping Table Address
This field points to the base of 4KB aligned interrupt remapping table.
Hardware may ignore and not implement bits 63:HAW, where HAW is the host
address width.

3:0 : Size
This field specifies the size of the interrupt remapping table. The
number of entries in the interrupt remapping table is 2 ^ X+1, where X is
the value programmed in this field.

And an Interrupt Remapping Table Entry (IRTE) takes 8 Bytes. So a 4KB page
would cover 256 Interrupt Remapping Table Entries (interrupt remapping table
takes a page at least).


Can we assume CONFIG_MAX_PT_IRQ_ENTRIES be in the power of 2 ? If yes, we can define both PT_IRQ_ENTRY_BITS, and MAX_PT_IRQ_ENTRIES= (1U << PT_IRQ_ENTRY_BITS). And refuse to boot if it is not.4
This way, it can be simplified.
Yes, but PT_IRQ_ENTRY_BITS maybe is not friendly to user, I think.

Could we still let user to configure CONFIG_MAX_PT_IRQ_ENTRIES, but let
config tool to help ACRN Hypervisor to do align check and calculate
the PT_IRQ_ENTRY_BITS ?
Config tool could count the PT_IRQ_ENTRY_BITS by CONFIG_MAX_PT_IRQ_ENTRIE.

This way, we could fix the issue and simple HV code.





So this patch split the powerof2_roundup to powerof2_roundup32 and
powerof2_roundup64 for different case.

Signed-off-by: Chenli Wei <chenli.wei@...>
---
hypervisor/include/arch/x86/asm/vtd.h | 2 +-
hypervisor/include/lib/util.h | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hypervisor/include/arch/x86/asm/vtd.h
b/hypervisor/include/arch/x86/asm/vtd.h
index 4d3518286..4df5b264b 100644
--- a/hypervisor/include/arch/x86/asm/vtd.h
+++ b/hypervisor/include/arch/x86/asm/vtd.h
@@ -46,7 +46,7 @@
#if (CONFIG_MAX_PT_IRQ_ENTRIES <= 256)
#define MAX_IR_ENTRIES 256
#else
-#define MAX_IR_ENTRIES
powerof2_roundup(CONFIG_MAX_PT_IRQ_ENTRIES)
+#define MAX_IR_ENTRIES
powerof2_roundup32(CONFIG_MAX_PT_IRQ_ENTRIES)
#endif

/* Values for entry_type in ACPI_DMAR_DEVICE_SCOPE - device types */
diff --git a/hypervisor/include/lib/util.h b/hypervisor/include/lib/util.h index
ca3296556..2e2a6c6df 100644
--- a/hypervisor/include/lib/util.h
+++ b/hypervisor/include/lib/util.h
@@ -44,7 +44,8 @@
#define ROUND8(x) (ROUND4(x) |(ROUND4(x)>>8)) #define ROUND16(x)
(ROUND8(x) |(ROUND8(x)>>16)) #define ROUND32(x) (ROUND16(x)
|(ROUND16(x)>>32)) -#define powerof2_roundup(x) ((ROUND32(x) ==
(~0UL)) ? ~0UL : (ROUND32(x) +1))
+#define powerof2_roundup32(x) ((ROUND16(x) == (~0UL)) ? ~0UL :
+(ROUND16(x) +1)) #define powerof2_roundup64(x) ((ROUND32(x) == (~0UL))
+? ~0UL : (ROUND32(x) +1))

/* Macro used to check if a value is aligned to the required boundary.
* Returns TRUE if aligned; FALSE if not aligned
--
2.25.1






Re: [PATCH] hv: fix the issue of "right shift count"

Li, Fei1
 

On Sat, May 07, 2022 at 06:09:02AM +0800, Dong, Eddie wrote:


-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On
Behalf Of chenli.wei
Sent: Friday, May 6, 2022 1:23 AM
To: Li, Fei1 <fei1.li@...>; acrn-dev@...
Cc: Wei, Chenli <chenli.wei@...>
Subject: [acrn-dev] [PATCH] hv: fix the issue of "right shift count"

The current code have use powerof2_roundup to get the 2^n, there was an
issue if used to get the 2^n of a macro, that's because macros have no data
type.
Didn't quite understand.


BTW, the current logic of powerof2_roundup() is very ugly, and I am wondering why we need the following logic:

#if (CONFIG_MAX_PT_IRQ_ENTRIES <= 256)
#define MAX_IR_ENTRIES 256
#else
#define MAX_IR_ENTRIES powerof2_roundup(CONFIG_MAX_PT_IRQ_ENTRIES)
#endif
Interrupt Remapping Table Address Register (IRTA_REG)

63:12 : Interrupt Remapping Table Address
This field points to the base of 4KB aligned interrupt remapping table.
Hardware may ignore and not implement bits 63:HAW, where HAW is the host
address width.

3:0 : Size
This field specifies the size of the interrupt remapping table. The
number of entries in the interrupt remapping table is 2 ^ X+1, where X is
the value programmed in this field.

And an Interrupt Remapping Table Entry (IRTE) takes 8 Bytes. So a 4KB page
would cover 256 Interrupt Remapping Table Entries (interrupt remapping table
takes a page at least).



Can we assume CONFIG_MAX_PT_IRQ_ENTRIES be in the power of 2 ? If yes, we can define both PT_IRQ_ENTRY_BITS, and MAX_PT_IRQ_ENTRIES= (1U << PT_IRQ_ENTRY_BITS). And refuse to boot if it is not.4
This way, it can be simplified.
Yes, but PT_IRQ_ENTRY_BITS maybe is not friendly to user, I think.

Could we still let user to configure CONFIG_MAX_PT_IRQ_ENTRIES, but let
config tool to help ACRN Hypervisor to do align check and calculate
the PT_IRQ_ENTRY_BITS ?






So this patch split the powerof2_roundup to powerof2_roundup32 and
powerof2_roundup64 for different case.

Signed-off-by: Chenli Wei <chenli.wei@...>
---
hypervisor/include/arch/x86/asm/vtd.h | 2 +-
hypervisor/include/lib/util.h | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hypervisor/include/arch/x86/asm/vtd.h
b/hypervisor/include/arch/x86/asm/vtd.h
index 4d3518286..4df5b264b 100644
--- a/hypervisor/include/arch/x86/asm/vtd.h
+++ b/hypervisor/include/arch/x86/asm/vtd.h
@@ -46,7 +46,7 @@
#if (CONFIG_MAX_PT_IRQ_ENTRIES <= 256)
#define MAX_IR_ENTRIES 256
#else
-#define MAX_IR_ENTRIES
powerof2_roundup(CONFIG_MAX_PT_IRQ_ENTRIES)
+#define MAX_IR_ENTRIES
powerof2_roundup32(CONFIG_MAX_PT_IRQ_ENTRIES)
#endif

/* Values for entry_type in ACPI_DMAR_DEVICE_SCOPE - device types */
diff --git a/hypervisor/include/lib/util.h b/hypervisor/include/lib/util.h index
ca3296556..2e2a6c6df 100644
--- a/hypervisor/include/lib/util.h
+++ b/hypervisor/include/lib/util.h
@@ -44,7 +44,8 @@
#define ROUND8(x) (ROUND4(x) |(ROUND4(x)>>8)) #define ROUND16(x)
(ROUND8(x) |(ROUND8(x)>>16)) #define ROUND32(x) (ROUND16(x)
|(ROUND16(x)>>32)) -#define powerof2_roundup(x) ((ROUND32(x) ==
(~0UL)) ? ~0UL : (ROUND32(x) +1))
+#define powerof2_roundup32(x) ((ROUND16(x) == (~0UL)) ? ~0UL :
+(ROUND16(x) +1)) #define powerof2_roundup64(x) ((ROUND32(x) == (~0UL))
+? ~0UL : (ROUND32(x) +1))

/* Macro used to check if a value is aligned to the required boundary.
* Returns TRUE if aligned; FALSE if not aligned
--
2.25.1





[PATCH v2] misc: configurator: Config usb mediator devices in dropdown list

Calvin Zhang <calvinzhang.cool@...>
 

Support detecting connected usb devices in board_inspector and list them
in the usb mediator configuration menu.

Tracked-On: #7424
Signed-off-by: Calvin Zhang <calvinzhang.cool@...>
---

Notes:
v2: move usb_device node under the acpi port device

.../board_inspector/extractors/95-usb.py | 38 +++++++++++++++++++
.../launch_config/launch_cfg_gen.py | 5 ++-
misc/config_tools/schema/VMtypes.xsd | 12 ++++++
misc/config_tools/schema/config.xsd | 10 +----
4 files changed, 54 insertions(+), 11 deletions(-)
create mode 100644 misc/config_tools/board_inspector/extractors/95-usb.py

diff --git a/misc/config_tools/board_inspector/extractors/95-usb.py b/misc/config_tools/board_inspector/extractors/95-usb.py
new file mode 100644
index 000000000..90ddac53c
--- /dev/null
+++ b/misc/config_tools/board_inspector/extractors/95-usb.py
@@ -0,0 +1,38 @@
+# Copyright (C) 2022 Intel Corporation. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-3-Clause
+#
+
+import os, re
+
+from extractors.helpers import add_child, get_node
+
+USB_DEVICES_PATH = "/sys/bus/usb/devices"
+USB_DEVICES_REGEX = r"^\d-\d$" # only devices connecting to root hub
+
+def extract(args, board_etree):
+ usb_devices_node = get_node(board_etree, f"//usb_devices")
+ if not usb_devices_node:
+ devices_node = get_node(board_etree, f"//devices")
+ usb_devices_node = add_child(devices_node, "usb_devices")
+
+ dev_regex = re.compile(USB_DEVICES_REGEX)
+ for dev in os.listdir(USB_DEVICES_PATH):
+ m = dev_regex.match(dev)
+ if m:
+ d = m.group(0)
+ devpath = os.path.join(USB_DEVICES_PATH, d)
+ with open(os.path.join(devpath, 'devnum'), 'r') as f:
+ devnum = f.read().strip()
+ with open(os.path.join(devpath, 'busnum'), 'r') as f:
+ busnum = f.read().strip()
+ cmd_out = os.popen('lsusb -s {b}:{d}'.format(b=busnum, d=devnum)).read()
+ desc = cmd_out.split(':', maxsplit=1)[1].strip('\n')
+
+ with open(devpath + '/port/firmware_node/path') as f:
+ acpi_path = f.read().strip()
+ usb_port_node = get_node(board_etree, f"//device[acpi_object='{acpi_path}']")
+ if usb_port_node is not None:
+ add_child(usb_port_node, "usb_device", location=d,
+ description=d + desc)
+
diff --git a/misc/config_tools/launch_config/launch_cfg_gen.py b/misc/config_tools/launch_config/launch_cfg_gen.py
index 302918e06..a168182e6 100755
--- a/misc/config_tools/launch_config/launch_cfg_gen.py
+++ b/misc/config_tools/launch_config/launch_cfg_gen.py
@@ -267,8 +267,9 @@ def generate_for_one_vm(board_etree, hv_scenario_etree, vm_scenario_etree, vm_id
script.add_virtual_device("uart", options="vuart_idx:{idx}")

# Mediated PCI devices, including virtio
- for usb_xhci in eval_xpath_all(vm_scenario_etree, ".//usb_xhci[text() != '']/text()"):
- script.add_virtual_device("xhci", options=usb_xhci)
+ for usb_xhci in eval_xpath_all(vm_scenario_etree, ".//usb_xhci/usb_dev[text() != '']/text()"):
+ bus_port = usb_xhci.split(' ')[0]
+ script.add_virtual_device("xhci", options=bus_port)

for virtio_input_etree in eval_xpath_all(vm_scenario_etree, ".//virtio_devices/input"):
backend_device_file = eval_xpath(virtio_input_etree, "./backend_device_file[text() != '']/text()")
diff --git a/misc/config_tools/schema/VMtypes.xsd b/misc/config_tools/schema/VMtypes.xsd
index f98c068d4..e03eb61a3 100644
--- a/misc/config_tools/schema/VMtypes.xsd
+++ b/misc/config_tools/schema/VMtypes.xsd
@@ -284,6 +284,18 @@ The size is a subset of the VM's total memory size specified on the Basic tab.</
</xs:sequence>
</xs:complexType>

+<xs:complexType name="USBDevsConfiguration">
+ <xs:sequence>
+ <xs:element name="usb_dev" type="xs:string" minOccurs="0" maxOccurs="unbounded">
+ <xs:annotation acrn:title="USB device assignment"
+ acrn:options="//usb_device/@description" acrn:options-sorted-by="lambda s: (s, s.split(' ')[0])">
+ <xs:documentation>Select the USB devices you want to assign to this virtual machine.</xs:documentation>
+ </xs:annotation>
+ </xs:element>
+ </xs:sequence>
+</xs:complexType>
+
+
<xs:simpleType name="VirtioNetworkFrameworkType">
<xs:annotation>
<xs:documentation>A string with value: ``Kernel based (Virtual Host)`` or ``User space based (VBSU)``.</xs:documentation>
diff --git a/misc/config_tools/schema/config.xsd b/misc/config_tools/schema/config.xsd
index 68cc575c6..2669b2a73 100644
--- a/misc/config_tools/schema/config.xsd
+++ b/misc/config_tools/schema/config.xsd
@@ -433,18 +433,10 @@ argument and memory.</xs:documentation>
<xs:documentation>Enable the ACRN Device Model to emulate COM1 as a User VM stdio I/O. Hypervisor global emulation will take priority over this VM setting.</xs:documentation>
</xs:annotation>
</xs:element>
- <xs:element name="usb_xhci" minOccurs="0">
+ <xs:element name="usb_xhci" type="USBDevsConfiguration" minOccurs="0">
<xs:annotation acrn:title="Virtual USB HCI" acrn:views="basic" acrn:applicable-vms="post-launched">
<xs:documentation>Select the USB physical bus and port number that will be emulated by the ACRN Device Model for this VM. USB 3.0, 2.0, and 1.0 are supported.</xs:documentation>
</xs:annotation>
- <xs:simpleType>
- <xs:annotation>
- <xs:documentation>Input format: ``bus#-port#[:bus#-port#: ...]``, for example, ``1-2:2-4``.</xs:documentation>
- </xs:annotation>
- <xs:restriction base="xs:string">
- <xs:pattern value="([\d]+-[\d]+){0,1}(:[\d]+-[\d]+)*" />
- </xs:restriction>
- </xs:simpleType>
</xs:element>
<xs:element name="virtio_devices">
<xs:annotation acrn:title="Virt-IO devices" acrn:views="basic" acrn:applicable-vms="post-launched">
--
2.30.2


Re: [PATCH v2 1/2] misc: refine CLOS module for expansibility

Junjie Mao
 

Chenli Wei <chenli.wei@...> writes:

This patch refine CLOS module by the following aspects:

1 Unified CACHE_ID type to Hex Format
2 Rewrite policy merge with RDT Class
3 Modify the logic of generate CPU mask

v1-->v2:
1. code format
2. add comment for RdtPolicy

Signed-off-by: Chenli Wei <chenli.wei@...>
---
misc/config_tools/board_config/board_c.py | 8 +-
misc/config_tools/schema/types.xsd | 2 +-
misc/config_tools/static_allocators/clos.py | 233 ++++++++++++--------
3 files changed, 144 insertions(+), 99 deletions(-)

diff --git a/misc/config_tools/board_config/board_c.py b/misc/config_tools/board_config/board_c.py
index 8082c4387..4be3ea4ec 100644
--- a/misc/config_tools/board_config/board_c.py
+++ b/misc/config_tools/board_config/board_c.py
@@ -173,7 +173,7 @@ def gen_rdt_str(cache, config):
return err_dic

cdp_enable = get_cdp_enabled()
- cat_mask_list = get_mask_list(cache_level, int(cache_id, 16))
+ cat_mask_list = get_mask_list(cache_level, cache_id)
if len(cat_mask_list) > int(clos_number):
err_dic['board config: Failed to generate board.c'] = "CLOS Mask Number too bigger then the supported of L2/L3 cache"
return err_dic;
@@ -207,7 +207,7 @@ def gen_rdt_str(cache, config):

cpu_mask = 0
for processor in processor_list:
- core_id = common.get_node(f"//core[@id = '{processor}']/thread/cpu_id/text()", board_etree)
+ core_id = common.get_node(f"//thread[apic_id = '{processor}']/cpu_id/text()", board_etree)
if core_id is None:
continue
else:
@@ -240,7 +240,7 @@ def gen_clos_array(cache_list, config):
clos_number = common.get_node(f"./capability/clos_number/text()", cache)
if cache_level == "2":

- cat_mask_list = get_mask_list(cache_level, int(cache_id, 16))
+ cat_mask_list = get_mask_list(cache_level, cache_id)
array_size = len(cat_mask_list)

print("union clos_config platform_l2_clos_array_{0}[{1}] = {{".format(int(cache_id, 16), clos_number), file=config)
@@ -250,7 +250,7 @@ def gen_clos_array(cache_list, config):
print("};\n", file=config)
res_present[RDT.L2.value] += 1
elif cache_level == "3":
- cat_mask_list = get_mask_list(cache_level, int(cache_id, 16))
+ cat_mask_list = get_mask_list(cache_level, cache_id)

print("union clos_config platform_l3_clos_array_{0}[{1}] = {{".format(int(cache_id, 16), clos_number), file=config)

diff --git a/misc/config_tools/schema/types.xsd b/misc/config_tools/schema/types.xsd
index 36a4aae09..cec4b84a5 100644
--- a/misc/config_tools/schema/types.xsd
+++ b/misc/config_tools/schema/types.xsd
@@ -355,7 +355,7 @@ RDT, setting this option to ``y`` is ignored.</xs:documentation>

<xs:complexType name="CacheAllocationType">
<xs:sequence>
- <xs:element name="CACHE_ID" type="xs:integer"/>
+ <xs:element name="CACHE_ID" type="HexFormat"/>
<xs:element name="CACHE_LEVEL" type="xs:integer"/>
<xs:element name="POLICY" type="CachePolicyType" minOccurs="1" maxOccurs="unbounded"/>
</xs:sequence>
diff --git a/misc/config_tools/static_allocators/clos.py b/misc/config_tools/static_allocators/clos.py
index 1f0476422..80b59435d 100644
--- a/misc/config_tools/static_allocators/clos.py
+++ b/misc/config_tools/static_allocators/clos.py
@@ -12,114 +12,159 @@ import re
from collections import defaultdict
from itertools import combinations

-def create_clos_node(etree, vm_id, index_list):
- allocation_vm_node = common.get_node(f"/acrn-config/vm[@id = '{vm_id}']", etree)
+class Identifier:
Use `namedtuple`. No need to write 10 lines only to revent a wheel.

Also, this class represents an identifier of what? Have you considered
CDP here as well?

+ vm_name = ""
+ vcpu = ""
+ cache_type = ""
+
+ def __init__(self, vm_name, vcpu, cache_type):
+ self.vm_name = vm_name
+ self.vcpu = vcpu
+ self.cache_type = cache_type
+
+ def __eq__(self, other):
+ return (self.vm_name == other.vm_name) and (self.vcpu == other.vcpu) and (self.cache_type == other.cache_type)
+
+class RdtPolicy:
+
+# a dictionary to save the CLOS policy from user setting
Indent the comments to the same level as the code you are commenting on.

+ policy_dict = {}
+
+# a list stored the vCPU or VM info
+ policy_owner = []
+
+# a list stored the L2 cache IDs
+ cache2_id_list = []
+
+#L2 cache have more then one section, this function find which one have been set
Typo: then -> than

+ def find_cache2_id(self, mask):
+ for cache2 in self.cache2_id_list:
+ if mask[cache2] != None:
+ return cache2
+ return None
+
+ def __init__(self, policy_dict, owner):
+ self.policy_dict = policy_dict
+ self.policy_owner = [owner]
+
+#check whether the src could be merged, if yes, add the src owner to policy_owner list and return True
+ def merge_policy(self, src):
+ if self.policy_dict["l3"] == src.policy_dict["l3"]:
While dicts in Python can use arbitrary keys (even with different
types), the code is much less maintainable if you do so.

If you want to wrap RDT policies as objects, do the following:

1. Instead of using an all-in-one dict, split L2 and L3 configurations
as different fields.

2. Add an interface that takes an XML element and updates the objects,
so that when walking through the policies you do not need to create
any more dictionaries or lists as another level of intermediate
representation.

+ cache2_id = self.find_cache2_id(src.policy_dict)
+ if (cache2_id == None) or (self.policy_dict[cache2_id] == src.policy_dict[cache2_id]):
+ self.policy_owner.append(src.policy_owner[0])
In theory you should extend `self.policy_owner` using
`src.policy_owner`. While today `src` may have at most one owner, it
relies on how you walk through the policies which this class had better
not make any assumption on.

+ return True
+ elif self.policy_dict[cache2_id] == None:
+ self.policy_dict[cache2_id] = src.policy_dict[cache2_id]
+ return True
+ return False
+
+#check whether a VM/vCPU could adapt this policy
+ def adapt_policy(self, policy_identifier):
"adapt" means "make sth. suitable", but this function is only a
predicate with no side effects.

+ for owner in self.policy_owner:
+ if owner == policy_identifier:
+ return True
+ return False
Do you mean `return policy_identifier in self.policy_owner` by the above
four lines?

+
+class vCatPolicy(RdtPolicy):
+ def merge_policy(self, src):
+ return False
+
+def create_clos_node(scenario_etree, vm_id, index_list):
+ allocation_vm_node = common.get_node(f"/acrn-config/vm[@id = '{vm_id}']", scenario_etree)
if allocation_vm_node is None:
- allocation_vm_node = common.append_node("/acrn-config/vm", None, etree, id = vm_id)
+ allocation_vm_node = common.append_node("/acrn-config/vm", None, scenario_etree, id = vm_id)
if common.get_node("./clos", allocation_vm_node) is None:
clos_node = common.append_node("./clos", None, allocation_vm_node)
for index in index_list:
common.append_node(f"./vcpu_clos", str(index), clos_node)

-def find_cache2_id(mask, cache2_id_list):
- for cache2 in cache2_id_list:
- if mask[cache2] != "None":
- return cache2
- return "None"
-
-def merge_policy_list(mask_list, cache2_id_list):
- index = 0
+def merge_policy_list(policy_list):
result_list = []
- for index,mask in enumerate(mask_list):
- merged = 0
+ for index,p in enumerate(policy_list):
+ merged = False
if index == 0:
Why do need this branch? When `index` is 0, `result_list` is empty and
`merged` will always be `False` after you walk through that empty
list. Adding this specific branch does little (if any) help to
performance but hurts a lot in readability.

- result_list.append(mask)
+ result_list.append(p)
continue
for result in result_list:
- if result["l3"] != mask["l3"]:
- continue
- else:
- cache2_id = find_cache2_id(mask, cache2_id_list)
- if cache2_id == "None" or result[cache2_id] == mask[cache2_id]:
- merged = 1
- break
- if result[cache2_id] == "None":
- merged = 1
- result[cache2_id] = mask[cache2_id]
- break
- if merged == 0:
- result_list.append(mask)
+ if result.merge_policy(p):
+ merged = True
+ break;
+ if merged == False:
Simply say `if not merged`.

+ result_list.append(p)
return result_list

-def gen_all_clos_index(board_etree, scenario_etree, allocation_etree):
- policy_list = []
- allocation_list = scenario_etree.xpath(f"//POLICY")
+def gen_identifier_list(scenario_etree):
+ identifier_list = []
+ vm_list = scenario_etree.xpath("//POLICY/VM")
+ for vm in vm_list:
+ vm_name = common.get_node("./text()", vm)
+ vcpu = common.get_node("../VCPU/text()", vm)
+ cache_type = common.get_node("../TYPE/text()", vm)
+ identifier_list.append(Identifier(vm_name, vcpu, cache_type))
+ return identifier_list
+
+def vm_vcat_enable(scenario_etree, vm_name):
+ vcat_enable = common.get_node(f"//VCAT_ENABLED/text()", scenario_etree)
+ virtual_cat_support = common.get_node(f"//vm[name = '{vm_name}']/virtual_cat_support/text()", scenario_etree)
+ return (vcat_enable == "y") and (virtual_cat_support == "y")
+
+def get_policy_list(board_etree, scenario_etree, allocation_etree):
cache2_id_list = scenario_etree.xpath("//CACHE_ALLOCATION[CACHE_LEVEL = 2]/CACHE_ID/text()")
cache2_id_list.sort()

- for policy in allocation_list:
- cache_level = common.get_node("../CACHE_LEVEL/text()", policy)
- cache_id = common.get_node("../CACHE_ID/text()", policy)
- vcpu = common.get_node("./VCPU/text()", policy)
- mask = common.get_node("./CLOS_MASK/text()", policy)
- tmp = (cache_level, cache_id, vcpu, mask)
- policy_list.append(tmp)
-
- vCPU_list = scenario_etree.xpath(f"//POLICY/VCPU/text()")
- l3_mask_list = scenario_etree.xpath(f"//CACHE_ALLOCATION[CACHE_LEVEL = 3]/POLICY/CLOS_MASK")
- mask_list = []
- for vCPU in vCPU_list:
+ RdtPolicy.cache2_id_list = cache2_id_list
+ identifier_list = gen_identifier_list(scenario_etree)
+
+ result_list = []
+ for ident in identifier_list:
dict_tmp = {}
- l3_mask = l2_mask = "None"
- l3_mask_list = scenario_etree.xpath(f"//CACHE_ALLOCATION[CACHE_LEVEL = 3]/POLICY[VCPU = '{vCPU}']/CLOS_MASK/text()")
- if len(l3_mask_list) > 0:
- l3_mask = l3_mask_list[0]
+ policy_list = scenario_etree.xpath(f"//POLICY[VM = '{ident.vm_name}' and VCPU = '{ident.vcpu}' and TYPE = '{ident.cache_type}']")
+ l3_mask = l2_mask = None
+ cache2_id = None
+ for policy in policy_list:
+ cache_level = common.get_node("../CACHE_LEVEL/text()", policy)
+ cache_id = common.get_node("../CACHE_ID/text()", policy)
+ clos_mask = common.get_node("./CLOS_MASK/text()", policy)
+ if cache_level == "2":
+ l2_mask = clos_mask
+ cache2_id = cache_id
+ else:
+ l3_mask = clos_mask
dict_tmp["l3"] = l3_mask
-
- l2_mask_list = scenario_etree.xpath(f"//CACHE_ALLOCATION[CACHE_LEVEL = 2]/POLICY[VCPU = '{vCPU}']/CLOS_MASK")
- if len(l2_mask_list) > 0:
- l2_mask = l2_mask_list[0].text
- cache_id = scenario_etree.xpath(f"//CACHE_ALLOCATION[CACHE_LEVEL = 2 and POLICY/VCPU = '{vCPU}']/CACHE_ID/text()")[0]
for cache2 in cache2_id_list:
- if cache2 == cache_id:
+ if cache2 == cache2_id:
dict_tmp[cache_id] = l2_mask
else:
- dict_tmp[cache2] = "None"
- mask_list.append(dict_tmp)
- mask_list = merge_policy_list(mask_list, cache2_id_list)
- return mask_list
-
-def get_clos_index(cache_level, cache_id, clos_mask):
- mask_list = common.get_mask_list(cache_level, cache_id)
- idx = 0
- for mask in mask_list:
- idx += 1
- if mask == clos_mask:
- break
- return idx
-def get_clos_id(mask_list, l2_id, l2_mask, l3_mask):
- for mask in mask_list:
- if mask[l2_id] == l2_mask and mask["l3"] == l3_mask:
- return mask_list.index(mask)
+ dict_tmp[cache2] = None
+ if vm_vcat_enable(scenario_etree, ident.vm_name):
+ result_list.append(vCatPolicy(dict_tmp, ident))
+ else:
+ result_list.append(RdtPolicy(dict_tmp, ident))
+ return merge_policy_list(result_list)
+
+def get_clos_id(rdt_list, mask_identifier):
+ for index,rdt in enumerate(rdt_list):
+ if rdt.adapt_policy(mask_identifier):
Please, be consistent with your names. What's the difference between
"identifier" and "owner"? If they are the same, unify the noun you use
to refer to them; otherwise, why you compare identifiers with owners in
the `adapt_policy` member function?

I'll stop here, as this patch still need a fundamental refactoring to
make the overall structure easy to read.

--
Best Regards
Junjie Mao

+ return index
return 0

def alloc_clos_index(board_etree, scenario_etree, allocation_etree, mask_list):
vm_node_list = scenario_etree.xpath("//vm")
for vm_node in vm_node_list:
- vmname = common.get_node("./name/text()", vm_node)
- allocation_list = scenario_etree.xpath(f"//CACHE_ALLOCATION[POLICY/VM = '{vmname}']")
- for allocation in allocation_list:
- index_list = []
- cache_level = common.get_node("./CACHE_LEVEL/text()", allocation)
- cache_id = common.get_node("./CACHE_ID/text()", allocation)
- clos_mask_list = allocation.xpath(f".//POLICY[VM = '{vmname}']/CLOS_MASK/text()")
-
- for clos_mask in clos_mask_list:
- index = get_clos_id(mask_list, cache_id, clos_mask, "None")
+ vm_name = common.get_node("./name/text()", vm_node)
+ vcpu_list = scenario_etree.xpath(f"//POLICY[VM = '{vm_name}']/VCPU/text()")
+ index_list = []
+ for vcpu in sorted(list(set(vcpu_list))):
+ type_list = scenario_etree.xpath(f"//POLICY[VM = '{vm_name}' and VCPU = '{vcpu}']/TYPE/text()")
+ for cache_type in sorted(list(set(type_list))):
+ if cache_type == "Data":
+ continue
+ index = get_clos_id(mask_list, Identifier(vm_name, vcpu, cache_type))
index_list.append(index)
- create_clos_node(allocation_etree, common.get_node("./@id", vm_node), index_list)
+ create_clos_node(allocation_etree, common.get_node("./@id", vm_node), index_list)

-def creat_mask_list_node(board_etree, scenario_etree, allocation_etree, mask_list):
+def create_mask_list_node(board_etree, scenario_etree, allocation_etree, rdt_policy_list):
allocation_hv_node = common.get_node(f"//hv", allocation_etree)
if allocation_hv_node is None:
allocation_hv_node = common.append_node(f"/acrn-config/hv", None, allocation_etree)
@@ -127,24 +172,24 @@ def creat_mask_list_node(board_etree, scenario_etree, allocation_etree, mask_lis
cache2_id_list.sort()
if common.get_node("./clos_mask[@id = l3]", allocation_hv_node) is None:
clos_mask = common.append_node("./clos_mask", None, allocation_hv_node, id="l3")
- for i in range(0, len(mask_list)):
- if mask_list[i]["l3"] == "None":
- value = "0xffff"
- else:
- value = str(mask_list[i]["l3"])
+ length = common.get_node(f"//cache[@level='3']/capability/capacity_mask_length/text()", board_etree)
+ value = hex((1 << int(length)) - 1)
+ for i in range(0, len(rdt_policy_list)):
+ if rdt_policy_list[i].policy_dict["l3"] is not None:
+ value = str(rdt_policy_list[i].policy_dict["l3"])
common.append_node(f"./clos", value, clos_mask)

for cache2 in cache2_id_list:
+ length = common.get_node(f"//cache[@level='2' and @id = '{cache2}']/capability/capacity_mask_length/text()", board_etree)
+ value = hex((1 << int(length)) - 1)
if common.get_node("./clos_mask[@id = '{cache2}']", allocation_hv_node) is None:
clos_mask = common.append_node("./clos_mask", None, allocation_hv_node, id=cache2)
- for i in range(0, len(mask_list)):
- if mask_list[i][cache2] == "None":
- value = "0xffff"
- else:
- value = str(mask_list[i][cache2] )
+ for i in range(0, len(rdt_policy_list)):
+ if rdt_policy_list[i].policy_dict[cache2] is not None:
+ value = str(rdt_policy_list[i].policy_dict[cache2] )
common.append_node(f"./clos", value, clos_mask)

def fn(board_etree, scenario_etree, allocation_etree):
- mask_list = gen_all_clos_index(board_etree, scenario_etree, allocation_etree)
- creat_mask_list_node(board_etree, scenario_etree, allocation_etree, mask_list)
- alloc_clos_index(board_etree, scenario_etree, allocation_etree, mask_list)
+ policy_list = get_policy_list(board_etree, scenario_etree, allocation_etree)
+ create_mask_list_node(board_etree, scenario_etree, allocation_etree, policy_list)
+ alloc_clos_index(board_etree, scenario_etree, allocation_etree, policy_list)


Re: [PATCH] hv: update SSRAM regions EPT memory type to WB

Li, Fei1
 

On Sat, May 07, 2022 at 04:40:51AM +0300, Yonghua Huang wrote:
when SSRAM regions are assigned to service VM
to support virtulization of SSRAM for post-launched
RTVMs, service VM need to access all SSRAM regions
for management, typically, service VM does data
cleanup in SSRAM region when it is reclaimed from
a shutdown RTVM.

This patch update memory type from UC(by default)
to WB.
You didn't commit why need to update this memory type.
Please update.


Tracked-On: #7425
Signed-off-by: Yonghua Huang <yonghua.huang@...>
---
hypervisor/arch/x86/guest/vm.c | 38 +++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/hypervisor/arch/x86/guest/vm.c b/hypervisor/arch/x86/guest/vm.c
index 786b390fa4e8..aff477abb0ac 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -538,16 +538,34 @@ static void prepare_service_vm_memmap(struct acrn_vm *vm)
pci_mmcfg = get_mmcfg_region();
ept_del_mr(vm, (uint64_t *)vm->arch_vm.nworld_eptp, pci_mmcfg->address, get_pci_mmcfg_size(pci_mmcfg));

- /* remove Software SRAM region from Service VM EPT, to prevent Service VM from using clflush to
- * flush the Software SRAM cache.
- * This is applicable to prelaunch RTVM case only. And only the part that prelaunch RTVM uses needs
- * to be removed from Service VM EPT.
- *
- * PRE_RTVM_SW_SRAM_MAX_SIZE is the size of Software SRAM that prelaunch RTVM uses, presumed to be
- * starting from Software SRAM base. For other cases, PRE_RTVM_SW_SRAM_MAX_SIZE should be defined
- * as 0, and no region is removed from Service VM EPT.
- */
- ept_del_mr(vm, pml4_page, service_vm_hpa2gpa(get_software_sram_base()), PRE_RTVM_SW_SRAM_MAX_SIZE);
+ if (is_software_sram_enabled()) {
+ /*
+ * Native Software SRAM resources shall be assigned to either Pre-launched RTVM
+ * or Service VM. Software SRAM support for Post-launched RTVM is virtualized
+ * in Service VM.
+ *
+ * 1) Native Software SRAM resources are assigned to Pre-launched RTVM:
+ * - Remove Software SRAM regions from Service VM EPT, to prevent
+ * Service VM from using clflush to flush the Software SRAM cache.
+ * - PRE_RTVM_SW_SRAM_MAX_SIZE is the size of Software SRAM that
+ * Pre-launched RTVM uses, presumed to be starting from Software SRAM base.
+ * For other cases, PRE_RTVM_SW_SRAM_MAX_SIZE should be defined as 0,
+ * and no region will be removed from Service VM EPT.
+ *
+ * 2) Native Software SRAM resources are assigned to Service VM:
+ * - Software SRAM regions are added to EPT of Service VM by default
+ * with memory type UC.
+ * - But, Service VM need to access Software SRAM regions
+ * when virtualizing them for Post-launched RTVM.
+ * - So memory type of Software SRAM regions in EPT shall be updated to EPT_WB.
+ */
+#if (PRE_RTVM_SW_SRAM_MAX_SIZE > 0U)
+ ept_del_mr(vm, pml4_page, service_vm_hpa2gpa(get_software_sram_base()), PRE_RTVM_SW_SRAM_MAX_SIZE);
+#else
+ ept_modify_mr(vm, pml4_page, service_vm_hpa2gpa(get_software_sram_base()),
+ get_software_sram_size(), EPT_WB, EPT_MT_MASK);
+#endif
+ }

/* unmap Intel IOMMU register pages for below reason:
* Service VM can detect IOMMU capability in its ACPI table hence it may access
--
2.25.1






[PATCH] hv: update SSRAM regions EPT memory type to WB

Yonghua Huang
 

when SSRAM regions are assigned to service VM
to support virtulization of SSRAM for post-launched
RTVMs, service VM need to access all SSRAM regions
for management, typically, service VM does data
cleanup in SSRAM region when it is reclaimed from
a shutdown RTVM.

This patch update memory type from UC(by default)
to WB.

Tracked-On: #7425
Signed-off-by: Yonghua Huang <yonghua.huang@...>
---
hypervisor/arch/x86/guest/vm.c | 38 +++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/hypervisor/arch/x86/guest/vm.c b/hypervisor/arch/x86/guest/vm.c
index 786b390fa4e8..aff477abb0ac 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -538,16 +538,34 @@ static void prepare_service_vm_memmap(struct acrn_vm *vm)
pci_mmcfg = get_mmcfg_region();
ept_del_mr(vm, (uint64_t *)vm->arch_vm.nworld_eptp, pci_mmcfg->address, get_pci_mmcfg_size(pci_mmcfg));

- /* remove Software SRAM region from Service VM EPT, to prevent Service VM from using clflush to
- * flush the Software SRAM cache.
- * This is applicable to prelaunch RTVM case only. And only the part that prelaunch RTVM uses needs
- * to be removed from Service VM EPT.
- *
- * PRE_RTVM_SW_SRAM_MAX_SIZE is the size of Software SRAM that prelaunch RTVM uses, presumed to be
- * starting from Software SRAM base. For other cases, PRE_RTVM_SW_SRAM_MAX_SIZE should be defined
- * as 0, and no region is removed from Service VM EPT.
- */
- ept_del_mr(vm, pml4_page, service_vm_hpa2gpa(get_software_sram_base()), PRE_RTVM_SW_SRAM_MAX_SIZE);
+ if (is_software_sram_enabled()) {
+ /*
+ * Native Software SRAM resources shall be assigned to either Pre-launched RTVM
+ * or Service VM. Software SRAM support for Post-launched RTVM is virtualized
+ * in Service VM.
+ *
+ * 1) Native Software SRAM resources are assigned to Pre-launched RTVM:
+ * - Remove Software SRAM regions from Service VM EPT, to prevent
+ * Service VM from using clflush to flush the Software SRAM cache.
+ * - PRE_RTVM_SW_SRAM_MAX_SIZE is the size of Software SRAM that
+ * Pre-launched RTVM uses, presumed to be starting from Software SRAM base.
+ * For other cases, PRE_RTVM_SW_SRAM_MAX_SIZE should be defined as 0,
+ * and no region will be removed from Service VM EPT.
+ *
+ * 2) Native Software SRAM resources are assigned to Service VM:
+ * - Software SRAM regions are added to EPT of Service VM by default
+ * with memory type UC.
+ * - But, Service VM need to access Software SRAM regions
+ * when virtualizing them for Post-launched RTVM.
+ * - So memory type of Software SRAM regions in EPT shall be updated to EPT_WB.
+ */
+#if (PRE_RTVM_SW_SRAM_MAX_SIZE > 0U)
+ ept_del_mr(vm, pml4_page, service_vm_hpa2gpa(get_software_sram_base()), PRE_RTVM_SW_SRAM_MAX_SIZE);
+#else
+ ept_modify_mr(vm, pml4_page, service_vm_hpa2gpa(get_software_sram_base()),
+ get_software_sram_size(), EPT_WB, EPT_MT_MASK);
+#endif
+ }

/* unmap Intel IOMMU register pages for below reason:
* Service VM can detect IOMMU capability in its ACPI table hence it may access
--
2.25.1


Re: [PATCH 2/2] ACRN:DM:VGPU: Add more standard modes for EDID block

Yu Wang
 

Acked-by: Wang, Yu1 <yu1.wang@...>

On Fri, May 06, 2022 at 11:39:25AM +0800, Zhao Yakui wrote:
EDID uses the bytes of 38-53 to support up to 8 standard modes. And it is
based on the mechanism: Byte 0 defines the xres / 8 - 31 and byte 1 defines
the aspect_ratio/refresh_rate.
But now it uses the incorrect logic in course of adding standard mode, which
causes that no standard mode is added in EDID block.

Fix it and add another two standard modes so that the guest_vm can parse
more modes from EDID.

Signed-off-by: Zhao Yakui <yakui.zhao@...>
---
devicemodel/hw/vdisplay_sdl.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/devicemodel/hw/vdisplay_sdl.c b/devicemodel/hw/vdisplay_sdl.c
index 02b8a6de1..eeefec17b 100644
--- a/devicemodel/hw/vdisplay_sdl.c
+++ b/devicemodel/hw/vdisplay_sdl.c
@@ -105,6 +105,7 @@ static const struct timing_entry {
uint32_t byte_t3;// byte idx in the Established Timings III Descriptor
uint32_t bit; // bit idx
uint8_t hz; // frequency
+ bool is_std; // the flag of standard mode
} timings[] = {
/* Established Timings I & II (all @ 60Hz) */
{ .hpixel = 1024, .vpixel = 768, .byte = 36, .bit = 3, .hz = 60},
@@ -112,8 +113,10 @@ static const struct timing_entry {
{ .hpixel = 640, .vpixel = 480, .byte = 35, .bit = 5, .hz = 60 },

/* Standard Timings */
- { .hpixel = 1920, .vpixel = 1080, .hz = 60 },
- { .hpixel = 1280, .vpixel = 720, .hz = 60 },
+ { .hpixel = 1920, .vpixel = 1080, .hz = 60, .is_std = true },
+ { .hpixel = 1280, .vpixel = 720, .hz = 60, .is_std = true },
+ { .hpixel = 1440, .vpixel = 900, .hz = 60, .is_std = true },
+ { .hpixel = 1680, .vpixel = 1050, .hz = 60, .is_std = true },
};

typedef struct frame_param{
@@ -269,7 +272,7 @@ vdpy_edid_set_timing(uint8_t *addr, const base_param *b_param, TIMING_MODE mode)
return;
}
case STDT: // Standard Timings
- if(stdcnt < 8 && (timing->hpixel == b_param->h_pixel)) {
+ if(stdcnt < 8 && timing->is_std) {
hpixel = (timing->hpixel >> 3) - 31;
if (timing->hpixel == 0 ||
timing->vpixel == 0) {
@@ -304,7 +307,7 @@ vdpy_edid_set_timing(uint8_t *addr, const base_param *b_param, TIMING_MODE mode)
}
break;
} else {
- return;
+ continue;
}

default:
--
2.25.1


Re: [PATCH 1/2] ACRN:DM:VGPU: Fix the incorrect offset for EDID detailed_timing_block

Yu Wang
 

Acked-by: Wang, Yu1 <yu1.wang@...>

On Fri, May 06, 2022 at 11:39:24AM +0800, Zhao Yakui wrote:
The EDID uses the bytes of 54-125 to define the four detailed_timing_blocks.
But the static offset is used to update the DTB. This will cause that
the incorrect offset is used for DTB if the edid_generate is called several
times.

Signed-off-by: Zhao Yakui <yakui.zhao@...>
---
devicemodel/hw/vdisplay_sdl.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/devicemodel/hw/vdisplay_sdl.c b/devicemodel/hw/vdisplay_sdl.c
index c29596788..02b8a6de1 100644
--- a/devicemodel/hw/vdisplay_sdl.c
+++ b/devicemodel/hw/vdisplay_sdl.c
@@ -353,22 +353,17 @@ vdpy_edid_set_dtd(uint8_t *dtd, const frame_param *frame)
}

static void
-vdpy_edid_set_descripter(uint8_t *edid, uint8_t is_dtd,
+vdpy_edid_set_descripter(uint8_t *desc, uint8_t is_dtd,
uint8_t tag, const base_param *b_param)
{
- static uint8_t offset;
- uint8_t *desc;
frame_param frame;
const char* text;
uint16_t len;

- offset = 54;
- desc = edid + offset;

if (is_dtd) {
vdpy_edid_set_frame(&frame, b_param);
vdpy_edid_set_dtd(desc, &frame);
- offset += 18;
return;
}
desc[3] = tag;
@@ -408,7 +403,6 @@ vdpy_edid_set_descripter(uint8_t *edid, uint8_t is_dtd,
default:
break;
}
- offset += 18;
}

static uint8_t
@@ -431,6 +425,7 @@ vdpy_edid_generate(uint8_t *edid, size_t size, struct edid_info *info)
uint16_t id_manuf;
uint16_t id_product;
uint32_t serial;
+ uint8_t *desc;
base_param b_param, c_param;

vdpy_edid_set_baseparam(&b_param, info->prefx, info->prefy);
@@ -496,15 +491,19 @@ vdpy_edid_generate(uint8_t *edid, size_t size, struct edid_info *info)

/* edid[125:54], Detailed Timing Descriptor - 18 bytes x 4 */
// Detailed Timing Descriptor 1
+ desc = edid + 54;
vdpy_edid_set_baseparam(&c_param, VDPY_MAX_WIDTH, VDPY_MAX_HEIGHT);
- vdpy_edid_set_descripter(edid, 0x1, 0, &c_param);
+ vdpy_edid_set_descripter(desc, 0x1, 0, &c_param);
// Detailed Timing Descriptor 2
+ desc += 18;
vdpy_edid_set_baseparam(&c_param, VDPY_DEFAULT_WIDTH, VDPY_DEFAULT_HEIGHT);
- vdpy_edid_set_descripter(edid, 0x1, 0, &c_param);
+ vdpy_edid_set_descripter(desc, 0x1, 0, &c_param);
// Display Product Name (ASCII) String Descriptor (tag #FCh)
- vdpy_edid_set_descripter(edid, 0, 0xfc, &b_param);
+ desc += 18;
+ vdpy_edid_set_descripter(desc, 0, 0xfc, &b_param);
// Display Product Serial Number Descriptor (tag #FFh)
- vdpy_edid_set_descripter(edid, 0, 0xff, &b_param);
+ desc += 18;
+ vdpy_edid_set_descripter(desc, 0, 0xff, &b_param);

/* EDID[126], Extension Block Count */
edid[126] = 0; // no Extension Block
--
2.25.1

1401 - 1420 of 37344