* Re: [PATCH net-next v2] net: mana: Allow variable size indirection table
From: Jakub Kicinski @ 2024-05-30 15:41 UTC (permalink / raw)
To: Shradha Gupta
Cc: linux-hardening, netdev, linux-hyperv, linux-kernel, linux-rdma,
Colin Ian King, Ahmed Zaki, Pavan Chebbi, Souradeep Chakrabarti,
Konstantin Taranov, Kees Cook, Paolo Abeni, Eric Dumazet,
David S. Miller, Dexuan Cui, Wei Liu, Haiyang Zhang,
K. Y. Srinivasan, Leon Romanovsky, Jason Gunthorpe, Ajay Sharma,
Long Li, Shradha Gupta
In-Reply-To: <1716960955-3195-1-git-send-email-shradhagupta@linux.microsoft.com>
On Tue, 28 May 2024 22:35:55 -0700 Shradha Gupta wrote:
> + save_table = kcalloc(apc->indir_table_sz, sizeof(u32), GFP_KERNEL);
> + if (!save_table)
> + return -ENOMEM;
> +
> if (rxfh->indir) {
> - for (i = 0; i < MANA_INDIRECT_TABLE_SIZE; i++)
> + for (i = 0; i < apc->indir_table_sz; i++)
> if (rxfh->indir[i] >= apc->num_queues)
> return -EINVAL;
leaks save_table
^ permalink raw reply
* Re: [PATCH net-next v2] net: mana: Allow variable size indirection table
From: Jakub Kicinski @ 2024-05-30 15:42 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Shradha Gupta, linux-hardening, netdev, linux-hyperv,
linux-kernel, linux-rdma, Colin Ian King, Ahmed Zaki,
Pavan Chebbi, Souradeep Chakrabarti, Konstantin Taranov,
Kees Cook, Paolo Abeni, Eric Dumazet, David S. Miller, Dexuan Cui,
Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Jason Gunthorpe,
Ajay Sharma, Long Li, Shradha Gupta
In-Reply-To: <20240530143702.GB3884@unreal>
On Thu, 30 May 2024 17:37:02 +0300 Leon Romanovsky wrote:
> Once you are ok with this patch, let me create shared branch for it.
> It is -rc1 and Konstantin already submitted some changes to qp.c
> https://lore.kernel.org/all/1716366242-558-1-git-send-email-kotaranov@linux.microsoft.com/
>
> This specific patch applies on top of Konstantin's changes cleanly.
Yeah, once it's not buggy shared branch SG! Just to be sure, on top
of -rc1, right?
^ permalink raw reply
* Re: [PATCH net-next v2] net: mana: Allow variable size indirection table
From: Leon Romanovsky @ 2024-05-30 16:03 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Shradha Gupta, linux-hardening, netdev, linux-hyperv,
linux-kernel, linux-rdma, Colin Ian King, Ahmed Zaki,
Pavan Chebbi, Souradeep Chakrabarti, Konstantin Taranov,
Kees Cook, Paolo Abeni, Eric Dumazet, David S. Miller, Dexuan Cui,
Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Jason Gunthorpe,
Ajay Sharma, Long Li, Shradha Gupta
In-Reply-To: <20240530084255.0b550d35@kernel.org>
On Thu, May 30, 2024 at 08:42:55AM -0700, Jakub Kicinski wrote:
> On Thu, 30 May 2024 17:37:02 +0300 Leon Romanovsky wrote:
> > Once you are ok with this patch, let me create shared branch for it.
> > It is -rc1 and Konstantin already submitted some changes to qp.c
> > https://lore.kernel.org/all/1716366242-558-1-git-send-email-kotaranov@linux.microsoft.com/
> >
> > This specific patch applies on top of Konstantin's changes cleanly.
>
> Yeah, once it's not buggy shared branch SG! Just to be sure, on top
> of -rc1, right?
Yes, it will be based on clean -rc1.
Thanks
^ permalink raw reply
* [PATCH v6] Drivers: hv: Cosmetic changes for hv.c and balloon.c
From: Aditya Nagesh @ 2024-05-31 10:48 UTC (permalink / raw)
To: adityanagesh, kys, haiyangz, wei.liu, decui, linux-hyperv,
linux-kernel
Cc: Aditya Nagesh
Fix issues reported by checkpatch.pl script in hv.c and
balloon.c
- Remove unnecessary parentheses
- Remove extra newlines
- Remove extra spaces
- Add spaces between comparison operators
- Remove comparison with NULL in if statements
No functional changes intended
Signed-off-by: Aditya Nagesh <adityanagesh@linux.microsoft.com>
Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
[V6]
Fix build failure and unintended change after rebase
[V5]
Rebase to hyperv-fixes
[V4]
Fix Alignment issue and revert a line since 100 characters are allowed in a line
[V3]
Fix alignment issues in multiline function parameters.
[V2]
Change Subject from "Drivers: hv: Fix Issues reported by checkpatch.pl script"
drivers/hv/hv.c | 37 ++++++++--------
drivers/hv/hv_balloon.c | 98 +++++++++++++++--------------------------
2 files changed, 53 insertions(+), 82 deletions(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index a8ad728354cb..e0d676c74f14 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -45,8 +45,8 @@ int hv_init(void)
* This involves a hypercall.
*/
int hv_post_message(union hv_connection_id connection_id,
- enum hv_message_type message_type,
- void *payload, size_t payload_size)
+ enum hv_message_type message_type,
+ void *payload, size_t payload_size)
{
struct hv_input_post_message *aligned_msg;
unsigned long flags;
@@ -86,7 +86,7 @@ int hv_post_message(union hv_connection_id connection_id,
status = HV_STATUS_INVALID_PARAMETER;
} else {
status = hv_do_hypercall(HVCALL_POST_MESSAGE,
- aligned_msg, NULL);
+ aligned_msg, NULL);
}
local_irq_restore(flags);
@@ -111,7 +111,7 @@ int hv_synic_alloc(void)
hv_context.hv_numa_map = kcalloc(nr_node_ids, sizeof(struct cpumask),
GFP_KERNEL);
- if (hv_context.hv_numa_map == NULL) {
+ if (!hv_context.hv_numa_map) {
pr_err("Unable to allocate NUMA map\n");
goto err;
}
@@ -120,11 +120,11 @@ int hv_synic_alloc(void)
hv_cpu = per_cpu_ptr(hv_context.cpu_context, cpu);
tasklet_init(&hv_cpu->msg_dpc,
- vmbus_on_msg_dpc, (unsigned long) hv_cpu);
+ vmbus_on_msg_dpc, (unsigned long)hv_cpu);
if (ms_hyperv.paravisor_present && hv_isolation_type_tdx()) {
hv_cpu->post_msg_page = (void *)get_zeroed_page(GFP_ATOMIC);
- if (hv_cpu->post_msg_page == NULL) {
+ if (!hv_cpu->post_msg_page) {
pr_err("Unable to allocate post msg page\n");
goto err;
}
@@ -147,14 +147,14 @@ int hv_synic_alloc(void)
if (!ms_hyperv.paravisor_present && !hv_root_partition) {
hv_cpu->synic_message_page =
(void *)get_zeroed_page(GFP_ATOMIC);
- if (hv_cpu->synic_message_page == NULL) {
+ if (!hv_cpu->synic_message_page) {
pr_err("Unable to allocate SYNIC message page\n");
goto err;
}
hv_cpu->synic_event_page =
(void *)get_zeroed_page(GFP_ATOMIC);
- if (hv_cpu->synic_event_page == NULL) {
+ if (!hv_cpu->synic_event_page) {
pr_err("Unable to allocate SYNIC event page\n");
free_page((unsigned long)hv_cpu->synic_message_page);
@@ -203,14 +203,13 @@ int hv_synic_alloc(void)
return ret;
}
-
void hv_synic_free(void)
{
int cpu, ret;
for_each_present_cpu(cpu) {
- struct hv_per_cpu_context *hv_cpu
- = per_cpu_ptr(hv_context.cpu_context, cpu);
+ struct hv_per_cpu_context *hv_cpu =
+ per_cpu_ptr(hv_context.cpu_context, cpu);
/* It's better to leak the page if the encryption fails. */
if (ms_hyperv.paravisor_present && hv_isolation_type_tdx()) {
@@ -262,8 +261,8 @@ void hv_synic_free(void)
*/
void hv_synic_enable_regs(unsigned int cpu)
{
- struct hv_per_cpu_context *hv_cpu
- = per_cpu_ptr(hv_context.cpu_context, cpu);
+ struct hv_per_cpu_context *hv_cpu =
+ per_cpu_ptr(hv_context.cpu_context, cpu);
union hv_synic_simp simp;
union hv_synic_siefp siefp;
union hv_synic_sint shared_sint;
@@ -277,8 +276,8 @@ void hv_synic_enable_regs(unsigned int cpu)
/* Mask out vTOM bit. ioremap_cache() maps decrypted */
u64 base = (simp.base_simp_gpa << HV_HYP_PAGE_SHIFT) &
~ms_hyperv.shared_gpa_boundary;
- hv_cpu->synic_message_page
- = (void *)ioremap_cache(base, HV_HYP_PAGE_SIZE);
+ hv_cpu->synic_message_page =
+ (void *)ioremap_cache(base, HV_HYP_PAGE_SIZE);
if (!hv_cpu->synic_message_page)
pr_err("Fail to map synic message page.\n");
} else {
@@ -296,8 +295,8 @@ void hv_synic_enable_regs(unsigned int cpu)
/* Mask out vTOM bit. ioremap_cache() maps decrypted */
u64 base = (siefp.base_siefp_gpa << HV_HYP_PAGE_SHIFT) &
~ms_hyperv.shared_gpa_boundary;
- hv_cpu->synic_event_page
- = (void *)ioremap_cache(base, HV_HYP_PAGE_SIZE);
+ hv_cpu->synic_event_page =
+ (void *)ioremap_cache(base, HV_HYP_PAGE_SIZE);
if (!hv_cpu->synic_event_page)
pr_err("Fail to map synic event page.\n");
} else {
@@ -348,8 +347,8 @@ int hv_synic_init(unsigned int cpu)
*/
void hv_synic_disable_regs(unsigned int cpu)
{
- struct hv_per_cpu_context *hv_cpu
- = per_cpu_ptr(hv_context.cpu_context, cpu);
+ struct hv_per_cpu_context *hv_cpu =
+ per_cpu_ptr(hv_context.cpu_context, cpu);
union hv_synic_sint shared_sint;
union hv_synic_simp simp;
union hv_synic_siefp siefp;
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 4370ad31b5b3..0e7427c2baf5 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -42,8 +42,6 @@
* Begin protocol definitions.
*/
-
-
/*
* Protocol versions. The low word is the minor version, the high word the major
* version.
@@ -72,8 +70,6 @@ enum {
DYNMEM_PROTOCOL_VERSION_CURRENT = DYNMEM_PROTOCOL_VERSION_WIN10
};
-
-
/*
* Message Types
*/
@@ -102,7 +98,6 @@ enum dm_message_type {
DM_VERSION_1_MAX = 12
};
-
/*
* Structures defining the dynamic memory management
* protocol.
@@ -116,7 +111,6 @@ union dm_version {
__u32 version;
} __packed;
-
union dm_caps {
struct {
__u64 balloon:1;
@@ -149,8 +143,6 @@ union dm_mem_page_range {
__u64 page_range;
} __packed;
-
-
/*
* The header for all dynamic memory messages:
*
@@ -175,7 +167,6 @@ struct dm_message {
__u8 data[]; /* enclosed message */
} __packed;
-
/*
* Specific message types supporting the dynamic memory protocol.
*/
@@ -272,7 +263,6 @@ struct dm_status {
__u32 io_diff;
} __packed;
-
/*
* Message to ask the guest to allocate memory - balloon up message.
* This message is sent from the host to the guest. The guest may not be
@@ -287,14 +277,13 @@ struct dm_balloon {
__u32 reservedz;
} __packed;
-
/*
* Balloon response message; this message is sent from the guest
* to the host in response to the balloon message.
*
* reservedz: Reserved; must be set to zero.
* more_pages: If FALSE, this is the last message of the transaction.
- * if TRUE there will atleast one more message from the guest.
+ * if TRUE there will be at least one more message from the guest.
*
* range_count: The number of ranges in the range array.
*
@@ -315,7 +304,7 @@ struct dm_balloon_response {
* to the guest to give guest more memory.
*
* more_pages: If FALSE, this is the last message of the transaction.
- * if TRUE there will atleast one more message from the guest.
+ * if TRUE there will be at least one more message from the guest.
*
* reservedz: Reserved; must be set to zero.
*
@@ -343,7 +332,6 @@ struct dm_unballoon_response {
struct dm_header hdr;
} __packed;
-
/*
* Hot add request message. Message sent from the host to the guest.
*
@@ -391,7 +379,6 @@ enum dm_info_type {
MAX_INFO_TYPE
};
-
/*
* Header for the information message.
*/
@@ -481,10 +468,10 @@ static unsigned long last_post_time;
static int hv_hypercall_multi_failure;
-module_param(hot_add, bool, (S_IRUGO | S_IWUSR));
+module_param(hot_add, bool, 0644);
MODULE_PARM_DESC(hot_add, "If set attempt memory hot_add");
-module_param(pressure_report_delay, uint, (S_IRUGO | S_IWUSR));
+module_param(pressure_report_delay, uint, 0644);
MODULE_PARM_DESC(pressure_report_delay, "Delay in secs in reporting pressure");
static atomic_t trans_id = ATOMIC_INIT(0);
@@ -503,7 +490,6 @@ enum hv_dm_state {
DM_INIT_ERROR
};
-
static __u8 recv_buffer[HV_HYP_PAGE_SIZE];
static __u8 balloon_up_send_buffer[HV_HYP_PAGE_SIZE];
@@ -599,12 +585,12 @@ static inline bool has_pfn_is_backed(struct hv_hotadd_state *has,
struct hv_hotadd_gap *gap;
/* The page is not backed. */
- if ((pfn < has->covered_start_pfn) || (pfn >= has->covered_end_pfn))
+ if (pfn < has->covered_start_pfn || pfn >= has->covered_end_pfn)
return false;
/* Check for gaps. */
list_for_each_entry(gap, &has->gap_list, list) {
- if ((pfn >= gap->start_pfn) && (pfn < gap->end_pfn))
+ if (pfn >= gap->start_pfn && pfn < gap->end_pfn)
return false;
}
@@ -784,8 +770,8 @@ static void hv_online_page(struct page *pg, unsigned int order)
guard(spinlock_irqsave)(&dm_device.ha_lock);
list_for_each_entry(has, &dm_device.ha_region_list, list) {
/* The page belongs to a different HAS. */
- if ((pfn < has->start_pfn) ||
- (pfn + (1UL << order) > has->end_pfn))
+ if (pfn < has->start_pfn ||
+ (pfn + (1UL << order) > has->end_pfn))
continue;
hv_bring_pgs_online(has, pfn, 1UL << order);
@@ -846,7 +832,7 @@ static int pfn_covered(unsigned long start_pfn, unsigned long pfn_cnt)
}
static unsigned long handle_pg_range(unsigned long pg_start,
- unsigned long pg_count)
+ unsigned long pg_count)
{
unsigned long start_pfn = pg_start;
unsigned long pfn_cnt = pg_count;
@@ -857,7 +843,7 @@ static unsigned long handle_pg_range(unsigned long pg_start,
unsigned long res = 0, flags;
pr_debug("Hot adding %lu pages starting at pfn 0x%lx.\n", pg_count,
- pg_start);
+ pg_start);
spin_lock_irqsave(&dm_device.ha_lock, flags);
list_for_each_entry(has, &dm_device.ha_region_list, list) {
@@ -893,10 +879,9 @@ static unsigned long handle_pg_range(unsigned long pg_start,
if (start_pfn > has->start_pfn &&
online_section_nr(pfn_to_section_nr(start_pfn)))
hv_bring_pgs_online(has, start_pfn, pgs_ol);
-
}
- if ((has->ha_end_pfn < has->end_pfn) && (pfn_cnt > 0)) {
+ if (has->ha_end_pfn < has->end_pfn && pfn_cnt > 0) {
/*
* We have some residual hot add range
* that needs to be hot added; hot add
@@ -999,7 +984,7 @@ static void hot_add_req(struct work_struct *dummy)
rg_start = dm->ha_wrk.ha_region_range.finfo.start_page;
rg_sz = dm->ha_wrk.ha_region_range.finfo.page_cnt;
- if ((rg_start == 0) && (!dm->host_specified_ha_region)) {
+ if (rg_start == 0 && !dm->host_specified_ha_region) {
/*
* The host has not specified the hot-add region.
* Based on the hot-add page range being specified,
@@ -1013,7 +998,7 @@ static void hot_add_req(struct work_struct *dummy)
if (do_hot_add)
resp.page_count = process_hot_add(pg_start, pfn_cnt,
- rg_start, rg_sz);
+ rg_start, rg_sz);
dm->num_pages_added += resp.page_count;
#endif
@@ -1191,11 +1176,10 @@ static void post_status(struct hv_dynmem_device *dm)
sizeof(struct dm_status),
(unsigned long)NULL,
VM_PKT_DATA_INBAND, 0);
-
}
static void free_balloon_pages(struct hv_dynmem_device *dm,
- union dm_mem_page_range *range_array)
+ union dm_mem_page_range *range_array)
{
int num_pages = range_array->finfo.page_cnt;
__u64 start_frame = range_array->finfo.start_page;
@@ -1211,8 +1195,6 @@ static void free_balloon_pages(struct hv_dynmem_device *dm,
}
}
-
-
static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
unsigned int num_pages,
struct dm_balloon_response *bl_resp,
@@ -1258,7 +1240,6 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
page_to_pfn(pg);
bl_resp->range_array[i].finfo.page_cnt = alloc_unit;
bl_resp->hdr.size += sizeof(union dm_mem_page_range);
-
}
return i * alloc_unit;
@@ -1312,7 +1293,7 @@ static void balloon_up(struct work_struct *dummy)
if (num_ballooned == 0 || num_ballooned == num_pages) {
pr_debug("Ballooned %u out of %u requested pages.\n",
- num_pages, dm_device.balloon_wrk.num_pages);
+ num_pages, dm_device.balloon_wrk.num_pages);
bl_resp->more_pages = 0;
done = true;
@@ -1346,16 +1327,15 @@ static void balloon_up(struct work_struct *dummy)
for (i = 0; i < bl_resp->range_count; i++)
free_balloon_pages(&dm_device,
- &bl_resp->range_array[i]);
+ &bl_resp->range_array[i]);
done = true;
}
}
-
}
static void balloon_down(struct hv_dynmem_device *dm,
- struct dm_unballoon_request *req)
+ struct dm_unballoon_request *req)
{
union dm_mem_page_range *range_array = req->range_array;
int range_count = req->range_count;
@@ -1369,7 +1349,7 @@ static void balloon_down(struct hv_dynmem_device *dm,
}
pr_debug("Freed %u ballooned pages.\n",
- prev_pages_ballooned - dm->num_pages_ballooned);
+ prev_pages_ballooned - dm->num_pages_ballooned);
if (req->more_pages == 1)
return;
@@ -1394,8 +1374,7 @@ static int dm_thread_func(void *dm_dev)
struct hv_dynmem_device *dm = dm_dev;
while (!kthread_should_stop()) {
- wait_for_completion_interruptible_timeout(
- &dm_device.config_event, 1*HZ);
+ wait_for_completion_interruptible_timeout(&dm_device.config_event, 1 * HZ);
/*
* The host expects us to post information on the memory
* pressure every second.
@@ -1419,9 +1398,8 @@ static int dm_thread_func(void *dm_dev)
return 0;
}
-
static void version_resp(struct hv_dynmem_device *dm,
- struct dm_version_response *vresp)
+ struct dm_version_response *vresp)
{
struct dm_version_request version_req;
int ret;
@@ -1482,7 +1460,7 @@ static void version_resp(struct hv_dynmem_device *dm,
}
static void cap_resp(struct hv_dynmem_device *dm,
- struct dm_capabilities_resp_msg *cap_resp)
+ struct dm_capabilities_resp_msg *cap_resp)
{
if (!cap_resp->is_accepted) {
pr_err("Capabilities not accepted by host\n");
@@ -1515,7 +1493,7 @@ static void balloon_onchannelcallback(void *context)
switch (dm_hdr->type) {
case DM_VERSION_RESPONSE:
version_resp(dm,
- (struct dm_version_response *)dm_msg);
+ (struct dm_version_response *)dm_msg);
break;
case DM_CAPABILITIES_RESPONSE:
@@ -1545,7 +1523,7 @@ static void balloon_onchannelcallback(void *context)
dm->state = DM_BALLOON_DOWN;
balloon_down(dm,
- (struct dm_unballoon_request *)recv_buffer);
+ (struct dm_unballoon_request *)recv_buffer);
break;
case DM_MEM_HOT_ADD_REQUEST:
@@ -1583,17 +1561,15 @@ static void balloon_onchannelcallback(void *context)
default:
pr_warn_ratelimited("Unhandled message: type: %d\n", dm_hdr->type);
-
}
}
-
}
#define HV_LARGE_REPORTING_ORDER 9
#define HV_LARGE_REPORTING_LEN (HV_HYP_PAGE_SIZE << \
HV_LARGE_REPORTING_ORDER)
static int hv_free_page_report(struct page_reporting_dev_info *pr_dev_info,
- struct scatterlist *sgl, unsigned int nents)
+ struct scatterlist *sgl, unsigned int nents)
{
unsigned long flags;
struct hv_memory_hint *hint;
@@ -1628,7 +1604,7 @@ static int hv_free_page_report(struct page_reporting_dev_info *pr_dev_info,
*/
/* page reporting for pages 2MB or higher */
- if (order >= HV_LARGE_REPORTING_ORDER ) {
+ if (order >= HV_LARGE_REPORTING_ORDER) {
range->page.largepage = 1;
range->page_size = HV_GPA_PAGE_RANGE_PAGE_SIZE_2MB;
range->base_large_pfn = page_to_hvpfn(
@@ -1642,23 +1618,21 @@ static int hv_free_page_report(struct page_reporting_dev_info *pr_dev_info,
range->page.additional_pages =
(sg->length / HV_HYP_PAGE_SIZE) - 1;
}
-
}
status = hv_do_rep_hypercall(HV_EXT_CALL_MEMORY_HEAT_HINT, nents, 0,
hint, NULL);
local_irq_restore(flags);
if (!hv_result_success(status)) {
-
pr_err("Cold memory discard hypercall failed with status %llx\n",
- status);
+ status);
if (hv_hypercall_multi_failure > 0)
hv_hypercall_multi_failure++;
if (hv_result(status) == HV_STATUS_INVALID_PARAMETER) {
pr_err("Underlying Hyper-V does not support order less than 9. Hypercall failed\n");
pr_err("Defaulting to page_reporting_order %d\n",
- pageblock_order);
+ pageblock_order);
page_reporting_order = pageblock_order;
hv_hypercall_multi_failure++;
return -EINVAL;
@@ -1692,7 +1666,7 @@ static void enable_page_reporting(void)
pr_err("Failed to enable cold memory discard: %d\n", ret);
} else {
pr_info("Cold memory discard hint enabled with order %d\n",
- page_reporting_order);
+ page_reporting_order);
}
}
@@ -1775,7 +1749,7 @@ static int balloon_connect_vsp(struct hv_device *dev)
if (ret)
goto out;
- t = wait_for_completion_timeout(&dm_device.host_event, 5*HZ);
+ t = wait_for_completion_timeout(&dm_device.host_event, 5 * HZ);
if (t == 0) {
ret = -ETIMEDOUT;
goto out;
@@ -1833,7 +1807,7 @@ static int balloon_connect_vsp(struct hv_device *dev)
if (ret)
goto out;
- t = wait_for_completion_timeout(&dm_device.host_event, 5*HZ);
+ t = wait_for_completion_timeout(&dm_device.host_event, 5 * HZ);
if (t == 0) {
ret = -ETIMEDOUT;
goto out;
@@ -1874,8 +1848,8 @@ static int hv_balloon_debug_show(struct seq_file *f, void *offset)
char *sname;
seq_printf(f, "%-22s: %u.%u\n", "host_version",
- DYNMEM_MAJOR_VERSION(dm->version),
- DYNMEM_MINOR_VERSION(dm->version));
+ DYNMEM_MAJOR_VERSION(dm->version),
+ DYNMEM_MINOR_VERSION(dm->version));
seq_printf(f, "%-22s:", "capabilities");
if (ballooning_enabled())
@@ -1924,10 +1898,10 @@ static int hv_balloon_debug_show(struct seq_file *f, void *offset)
seq_printf(f, "%-22s: %u\n", "pages_ballooned", dm->num_pages_ballooned);
seq_printf(f, "%-22s: %lu\n", "total_pages_committed",
- get_pages_committed(dm));
+ get_pages_committed(dm));
seq_printf(f, "%-22s: %llu\n", "max_dynamic_page_count",
- dm->max_dynamic_page_count);
+ dm->max_dynamic_page_count);
return 0;
}
@@ -1937,7 +1911,7 @@ DEFINE_SHOW_ATTRIBUTE(hv_balloon_debug);
static void hv_balloon_debugfs_init(struct hv_dynmem_device *b)
{
debugfs_create_file("hv-balloon", 0444, NULL, b,
- &hv_balloon_debug_fops);
+ &hv_balloon_debug_fops);
}
static void hv_balloon_debugfs_exit(struct hv_dynmem_device *b)
@@ -2095,7 +2069,6 @@ static int balloon_suspend(struct hv_device *hv_dev)
tasklet_enable(&hv_dev->channel->callback_event);
return 0;
-
}
static int balloon_resume(struct hv_device *dev)
@@ -2154,7 +2127,6 @@ static struct hv_driver balloon_drv = {
static int __init init_balloon_drv(void)
{
-
return vmbus_driver_register(&balloon_drv);
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCHv11 11/19] x86/tdx: Convert shared memory back to private on kexec
From: Borislav Petkov @ 2024-05-31 15:14 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, x86, Rafael J. Wysocki,
Peter Zijlstra, Adrian Hunter, Kuppuswamy Sathyanarayanan,
Elena Reshetova, Jun Nakajima, Rick Edgecombe, Tom Lendacky,
Kalra, Ashish, Sean Christopherson, Huang, Kai, Ard Biesheuvel,
Baoquan He, H. Peter Anvin, K. Y. Srinivasan, Haiyang Zhang,
kexec, linux-hyperv, linux-acpi, linux-coco, linux-kernel,
Tao Liu
In-Reply-To: <20240528095522.509667-12-kirill.shutemov@linux.intel.com>
On Tue, May 28, 2024 at 12:55:14PM +0300, Kirill A. Shutemov wrote:
> +static void tdx_kexec_finish(void)
> +{
> + unsigned long addr, end;
> + long found = 0, shared;
> +
> + lockdep_assert_irqs_disabled();
> +
> + addr = PAGE_OFFSET;
> + end = PAGE_OFFSET + get_max_mapped();
> +
> + while (addr < end) {
> + unsigned long size;
> + unsigned int level;
> + pte_t *pte;
> +
> + pte = lookup_address(addr, &level);
> + size = page_level_size(level);
> +
> + if (pte && pte_decrypted(*pte)) {
> + int pages = size / PAGE_SIZE;
> +
> + /*
> + * Touching memory with shared bit set triggers implicit
> + * conversion to shared.
> + *
> + * Make sure nobody touches the shared range from
> + * now on.
> + */
> + set_pte(pte, __pte(0));
> +
Format the below into a comment here:
/*
The only thing one can do at this point on failure is panic. It is
reasonable to proceed, especially for the crash case because the
kexec-ed kernel is using a different page table so there won't be
a mismatch between shared/private marking of the page so it doesn't
matter.
Also, even if the failure is real and the page cannot be touched as
private, the kdump kernel will boot fine as it uses pre-reserved memory.
What happens next depends on what the dumping process does and there's
a reasonable chance to produce useful dump on crash.
Regardless, the print leaves a trace in the log to give a clue for
debug.
One possible reason for the failure is if kdump raced with memory
conversion. In this case shared bit in page table got set (or not
cleared form shared->private conversion), but the page is actually
private. So this failure is not going to affect the kexec'ed kernel.
*/
<---
> + if (!tdx_enc_status_changed(addr, pages, true)) {
> + pr_err("Failed to unshare range %#lx-%#lx\n",
> + addr, addr + size);
> + }
> +
> + found += pages;
> + }
> +
> + addr += size;
> + }
> +
> + __flush_tlb_all();
> +
> + shared = atomic_long_read(&nr_shared);
> + if (shared != found) {
> + pr_err("shared page accounting is off\n");
> + pr_err("nr_shared = %ld, nr_found = %ld\n", shared, found);
> + }
> +}
...
> static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
> {
> - if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
> - return __set_memory_enc_pgtable(addr, numpages, enc);
> + int ret = 0;
>
> - return 0;
> + if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
> + if (!down_read_trylock(&mem_enc_lock))
> + return -EBUSY;
> +
> + ret = __set_memory_enc_pgtable(addr, numpages, enc);
> +
> + up_read(&mem_enc_lock);
> + }
So CC_ATTR_MEM_ENCRYPT is set for SEV* guests too. You need to change
that code here to take the lock only on TDX, where you want it, not on
the others.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* [PATCH net-next v3] net: mana: Allow variable size indirection table
From: Shradha Gupta @ 2024-05-31 15:37 UTC (permalink / raw)
To: linux-hardening, netdev, linux-hyperv, linux-kernel, linux-rdma
Cc: Shradha Gupta, Colin Ian King, Ahmed Zaki, Pavan Chebbi,
Souradeep Chakrabarti, Konstantin Taranov, Kees Cook, Paolo Abeni,
Jakub Kicinski, Eric Dumazet, David S. Miller, Dexuan Cui,
Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Leon Romanovsky,
Jason Gunthorpe, Long Li, Shradha Gupta
Allow variable size indirection table allocation in MANA instead
of using a constant value MANA_INDIRECT_TABLE_SIZE.
The size is now derived from the MANA_QUERY_VPORT_CONFIG and the
indirection table is allocated dynamically.
Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
Reviewed-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
Changes in v3:
* Fixed the memory leak(save_table) in mana_set_rxfh()
Changes in v2:
* Rebased to latest net-next tree
* Rearranged cleanup code in mana_probe_port to avoid extra operations
---
drivers/infiniband/hw/mana/qp.c | 10 +--
drivers/net/ethernet/microsoft/mana/mana_en.c | 68 ++++++++++++++++---
.../ethernet/microsoft/mana/mana_ethtool.c | 27 +++++---
include/net/mana/gdma.h | 4 +-
include/net/mana/mana.h | 9 +--
5 files changed, 89 insertions(+), 29 deletions(-)
diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
index ba13c5abf8ef..2d411a16a127 100644
--- a/drivers/infiniband/hw/mana/qp.c
+++ b/drivers/infiniband/hw/mana/qp.c
@@ -21,7 +21,7 @@ static int mana_ib_cfg_vport_steering(struct mana_ib_dev *dev,
gc = mdev_to_gc(dev);
- req_buf_size = struct_size(req, indir_tab, MANA_INDIRECT_TABLE_SIZE);
+ req_buf_size = struct_size(req, indir_tab, MANA_INDIRECT_TABLE_DEF_SIZE);
req = kzalloc(req_buf_size, GFP_KERNEL);
if (!req)
return -ENOMEM;
@@ -41,18 +41,18 @@ static int mana_ib_cfg_vport_steering(struct mana_ib_dev *dev,
if (log_ind_tbl_size)
req->rss_enable = true;
- req->num_indir_entries = MANA_INDIRECT_TABLE_SIZE;
+ req->num_indir_entries = MANA_INDIRECT_TABLE_DEF_SIZE;
req->indir_tab_offset = offsetof(struct mana_cfg_rx_steer_req_v2,
indir_tab);
req->update_indir_tab = true;
req->cqe_coalescing_enable = 1;
/* The ind table passed to the hardware must have
- * MANA_INDIRECT_TABLE_SIZE entries. Adjust the verb
+ * MANA_INDIRECT_TABLE_DEF_SIZE entries. Adjust the verb
* ind_table to MANA_INDIRECT_TABLE_SIZE if required
*/
ibdev_dbg(&dev->ib_dev, "ind table size %u\n", 1 << log_ind_tbl_size);
- for (i = 0; i < MANA_INDIRECT_TABLE_SIZE; i++) {
+ for (i = 0; i < MANA_INDIRECT_TABLE_DEF_SIZE; i++) {
req->indir_tab[i] = ind_table[i % (1 << log_ind_tbl_size)];
ibdev_dbg(&dev->ib_dev, "index %u handle 0x%llx\n", i,
req->indir_tab[i]);
@@ -137,7 +137,7 @@ static int mana_ib_create_qp_rss(struct ib_qp *ibqp, struct ib_pd *pd,
}
ind_tbl_size = 1 << ind_tbl->log_ind_tbl_size;
- if (ind_tbl_size > MANA_INDIRECT_TABLE_SIZE) {
+ if (ind_tbl_size > MANA_INDIRECT_TABLE_DEF_SIZE) {
ibdev_dbg(&mdev->ib_dev,
"Indirect table size %d exceeding limit\n",
ind_tbl_size);
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index d087cf954f75..851e1b9761b3 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -481,7 +481,7 @@ static int mana_get_tx_queue(struct net_device *ndev, struct sk_buff *skb,
struct sock *sk = skb->sk;
int txq;
- txq = apc->indir_table[hash & MANA_INDIRECT_TABLE_MASK];
+ txq = apc->indir_table[hash & (apc->indir_table_sz - 1)];
if (txq != old_q && sk && sk_fullsock(sk) &&
rcu_access_pointer(sk->sk_dst_cache))
@@ -962,7 +962,16 @@ static int mana_query_vport_cfg(struct mana_port_context *apc, u32 vport_index,
*max_sq = resp.max_num_sq;
*max_rq = resp.max_num_rq;
- *num_indir_entry = resp.num_indirection_ent;
+ if (resp.num_indirection_ent > 0 &&
+ resp.num_indirection_ent <= MANA_INDIRECT_TABLE_MAX_SIZE &&
+ is_power_of_2(resp.num_indirection_ent)) {
+ *num_indir_entry = resp.num_indirection_ent;
+ } else {
+ netdev_warn(apc->ndev,
+ "Setting indirection table size to default %d for vPort %d\n",
+ MANA_INDIRECT_TABLE_DEF_SIZE, apc->port_idx);
+ *num_indir_entry = MANA_INDIRECT_TABLE_DEF_SIZE;
+ }
apc->port_handle = resp.vport;
ether_addr_copy(apc->mac_addr, resp.mac_addr);
@@ -1054,14 +1063,13 @@ static int mana_cfg_vport_steering(struct mana_port_context *apc,
bool update_default_rxobj, bool update_key,
bool update_tab)
{
- u16 num_entries = MANA_INDIRECT_TABLE_SIZE;
struct mana_cfg_rx_steer_req_v2 *req;
struct mana_cfg_rx_steer_resp resp = {};
struct net_device *ndev = apc->ndev;
u32 req_buf_size;
int err;
- req_buf_size = struct_size(req, indir_tab, num_entries);
+ req_buf_size = struct_size(req, indir_tab, apc->indir_table_sz);
req = kzalloc(req_buf_size, GFP_KERNEL);
if (!req)
return -ENOMEM;
@@ -1072,7 +1080,7 @@ static int mana_cfg_vport_steering(struct mana_port_context *apc,
req->hdr.req.msg_version = GDMA_MESSAGE_V2;
req->vport = apc->port_handle;
- req->num_indir_entries = num_entries;
+ req->num_indir_entries = apc->indir_table_sz;
req->indir_tab_offset = offsetof(struct mana_cfg_rx_steer_req_v2,
indir_tab);
req->rx_enable = rx;
@@ -1111,7 +1119,7 @@ static int mana_cfg_vport_steering(struct mana_port_context *apc,
}
netdev_info(ndev, "Configured steering vPort %llu entries %u\n",
- apc->port_handle, num_entries);
+ apc->port_handle, apc->indir_table_sz);
out:
kfree(req);
return err;
@@ -2344,11 +2352,33 @@ static int mana_create_vport(struct mana_port_context *apc,
return mana_create_txq(apc, net);
}
+static int mana_rss_table_alloc(struct mana_port_context *apc)
+{
+ if (!apc->indir_table_sz) {
+ netdev_err(apc->ndev,
+ "Indirection table size not set for vPort %d\n",
+ apc->port_idx);
+ return -EINVAL;
+ }
+
+ apc->indir_table = kcalloc(apc->indir_table_sz, sizeof(u32), GFP_KERNEL);
+ if (!apc->indir_table)
+ return -ENOMEM;
+
+ apc->rxobj_table = kcalloc(apc->indir_table_sz, sizeof(mana_handle_t), GFP_KERNEL);
+ if (!apc->rxobj_table) {
+ kfree(apc->indir_table);
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
static void mana_rss_table_init(struct mana_port_context *apc)
{
int i;
- for (i = 0; i < MANA_INDIRECT_TABLE_SIZE; i++)
+ for (i = 0; i < apc->indir_table_sz; i++)
apc->indir_table[i] =
ethtool_rxfh_indir_default(i, apc->num_queues);
}
@@ -2361,7 +2391,7 @@ int mana_config_rss(struct mana_port_context *apc, enum TRI_STATE rx,
int i;
if (update_tab) {
- for (i = 0; i < MANA_INDIRECT_TABLE_SIZE; i++) {
+ for (i = 0; i < apc->indir_table_sz; i++) {
queue_idx = apc->indir_table[i];
apc->rxobj_table[i] = apc->rxqs[queue_idx]->rxobj;
}
@@ -2466,7 +2496,6 @@ static int mana_init_port(struct net_device *ndev)
struct mana_port_context *apc = netdev_priv(ndev);
u32 max_txq, max_rxq, max_queues;
int port_idx = apc->port_idx;
- u32 num_indirect_entries;
int err;
err = mana_init_port_context(apc);
@@ -2474,7 +2503,7 @@ static int mana_init_port(struct net_device *ndev)
return err;
err = mana_query_vport_cfg(apc, port_idx, &max_txq, &max_rxq,
- &num_indirect_entries);
+ &apc->indir_table_sz);
if (err) {
netdev_err(ndev, "Failed to query info for vPort %d\n",
port_idx);
@@ -2723,6 +2752,10 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
if (err)
goto free_net;
+ err = mana_rss_table_alloc(apc);
+ if (err)
+ goto reset_apc;
+
netdev_lockdep_set_classes(ndev);
ndev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
@@ -2739,11 +2772,17 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
err = register_netdev(ndev);
if (err) {
netdev_err(ndev, "Unable to register netdev.\n");
- goto reset_apc;
+ goto free_indir;
}
return 0;
+free_indir:
+ apc->indir_table_sz = 0;
+ kfree(apc->indir_table);
+ apc->indir_table = NULL;
+ kfree(apc->rxobj_table);
+ apc->rxobj_table = NULL;
reset_apc:
kfree(apc->rxqs);
apc->rxqs = NULL;
@@ -2897,6 +2936,7 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
{
struct gdma_context *gc = gd->gdma_context;
struct mana_context *ac = gd->driver_data;
+ struct mana_port_context *apc;
struct device *dev = gc->dev;
struct net_device *ndev;
int err;
@@ -2908,6 +2948,7 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
for (i = 0; i < ac->num_ports; i++) {
ndev = ac->ports[i];
+ apc = netdev_priv(ndev);
if (!ndev) {
if (i == 0)
dev_err(dev, "No net device to remove\n");
@@ -2931,6 +2972,11 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
}
unregister_netdevice(ndev);
+ apc->indir_table_sz = 0;
+ kfree(apc->indir_table);
+ apc->indir_table = NULL;
+ kfree(apc->rxobj_table);
+ apc->rxobj_table = NULL;
rtnl_unlock();
diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
index ab2413d71f6c..146d5db1792f 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
@@ -245,7 +245,9 @@ static u32 mana_get_rxfh_key_size(struct net_device *ndev)
static u32 mana_rss_indir_size(struct net_device *ndev)
{
- return MANA_INDIRECT_TABLE_SIZE;
+ struct mana_port_context *apc = netdev_priv(ndev);
+
+ return apc->indir_table_sz;
}
static int mana_get_rxfh(struct net_device *ndev,
@@ -257,7 +259,7 @@ static int mana_get_rxfh(struct net_device *ndev,
rxfh->hfunc = ETH_RSS_HASH_TOP; /* Toeplitz */
if (rxfh->indir) {
- for (i = 0; i < MANA_INDIRECT_TABLE_SIZE; i++)
+ for (i = 0; i < apc->indir_table_sz; i++)
rxfh->indir[i] = apc->indir_table[i];
}
@@ -273,8 +275,8 @@ static int mana_set_rxfh(struct net_device *ndev,
{
struct mana_port_context *apc = netdev_priv(ndev);
bool update_hash = false, update_table = false;
- u32 save_table[MANA_INDIRECT_TABLE_SIZE];
u8 save_key[MANA_HASH_KEY_SIZE];
+ u32 *save_table;
int i, err;
if (!apc->port_is_up)
@@ -284,13 +286,19 @@ static int mana_set_rxfh(struct net_device *ndev,
rxfh->hfunc != ETH_RSS_HASH_TOP)
return -EOPNOTSUPP;
+ save_table = kcalloc(apc->indir_table_sz, sizeof(u32), GFP_KERNEL);
+ if (!save_table)
+ return -ENOMEM;
+
if (rxfh->indir) {
- for (i = 0; i < MANA_INDIRECT_TABLE_SIZE; i++)
- if (rxfh->indir[i] >= apc->num_queues)
- return -EINVAL;
+ for (i = 0; i < apc->indir_table_sz; i++)
+ if (rxfh->indir[i] >= apc->num_queues) {
+ err = -EINVAL;
+ goto cleanup;
+ }
update_table = true;
- for (i = 0; i < MANA_INDIRECT_TABLE_SIZE; i++) {
+ for (i = 0; i < apc->indir_table_sz; i++) {
save_table[i] = apc->indir_table[i];
apc->indir_table[i] = rxfh->indir[i];
}
@@ -306,7 +314,7 @@ static int mana_set_rxfh(struct net_device *ndev,
if (err) { /* recover to original values */
if (update_table) {
- for (i = 0; i < MANA_INDIRECT_TABLE_SIZE; i++)
+ for (i = 0; i < apc->indir_table_sz; i++)
apc->indir_table[i] = save_table[i];
}
@@ -316,6 +324,9 @@ static int mana_set_rxfh(struct net_device *ndev,
mana_config_rss(apc, TRI_STATE_TRUE, update_hash, update_table);
}
+cleanup:
+ kfree(save_table);
+
return err;
}
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 27684135bb4d..c547756c4284 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -543,11 +543,13 @@ enum {
*/
#define GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX BIT(2)
#define GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECONFIG BIT(3)
+#define GDMA_DRV_CAP_FLAG_1_VARIABLE_INDIRECTION_TABLE_SUPPORT BIT(5)
#define GDMA_DRV_CAP_FLAGS1 \
(GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
- GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECONFIG)
+ GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECONFIG | \
+ GDMA_DRV_CAP_FLAG_1_VARIABLE_INDIRECTION_TABLE_SUPPORT)
#define GDMA_DRV_CAP_FLAGS2 0
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 561f6719fb4e..59823901b74f 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -30,8 +30,8 @@ enum TRI_STATE {
};
/* Number of entries for hardware indirection table must be in power of 2 */
-#define MANA_INDIRECT_TABLE_SIZE 64
-#define MANA_INDIRECT_TABLE_MASK (MANA_INDIRECT_TABLE_SIZE - 1)
+#define MANA_INDIRECT_TABLE_MAX_SIZE 512
+#define MANA_INDIRECT_TABLE_DEF_SIZE 64
/* The Toeplitz hash key's length in bytes: should be multiple of 8 */
#define MANA_HASH_KEY_SIZE 40
@@ -410,10 +410,11 @@ struct mana_port_context {
struct mana_tx_qp *tx_qp;
/* Indirection Table for RX & TX. The values are queue indexes */
- u32 indir_table[MANA_INDIRECT_TABLE_SIZE];
+ u32 *indir_table;
+ u32 indir_table_sz;
/* Indirection table containing RxObject Handles */
- mana_handle_t rxobj_table[MANA_INDIRECT_TABLE_SIZE];
+ mana_handle_t *rxobj_table;
/* Hash key used by the NIC */
u8 hashkey[MANA_HASH_KEY_SIZE];
--
2.34.1
^ permalink raw reply related
* Re: [PATCHv11 11/19] x86/tdx: Convert shared memory back to private on kexec
From: Kalra, Ashish @ 2024-05-31 17:34 UTC (permalink / raw)
To: Borislav Petkov, Kirill A. Shutemov
Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, x86, Rafael J. Wysocki,
Peter Zijlstra, Adrian Hunter, Kuppuswamy Sathyanarayanan,
Elena Reshetova, Jun Nakajima, Rick Edgecombe, Tom Lendacky,
Sean Christopherson, Huang, Kai, Ard Biesheuvel, Baoquan He,
H. Peter Anvin, K. Y. Srinivasan, Haiyang Zhang, kexec,
linux-hyperv, linux-acpi, linux-coco, linux-kernel, Tao Liu
In-Reply-To: <20240531151442.GMZlnpYkDCRlg1_YS0@fat_crate.local>
Hello Boris,
On 5/31/2024 10:14 AM, Borislav Petkov wrote:
>> static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
>> {
>> - if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
>> - return __set_memory_enc_pgtable(addr, numpages, enc);
>> + int ret = 0;
>>
>> - return 0;
>> + if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
>> + if (!down_read_trylock(&mem_enc_lock))
>> + return -EBUSY;
>> +
>> + ret = __set_memory_enc_pgtable(addr, numpages, enc);
>> +
>> + up_read(&mem_enc_lock);
>> + }
> So CC_ATTR_MEM_ENCRYPT is set for SEV* guests too. You need to change
> that code here to take the lock only on TDX, where you want it, not on
> the others.
SNP guest kexec patches are based on top of this patch-series and SNP
guests also need this exclusive mem_enc_lock protection, so
CC_ATTR_MEM_ENCRYPT makes sense to be used here.
Thanks, Ashish
^ permalink raw reply
* Re: [PATCHv11 11/19] x86/tdx: Convert shared memory back to private on kexec
From: Borislav Petkov @ 2024-05-31 18:06 UTC (permalink / raw)
To: Kalra, Ashish
Cc: Kirill A. Shutemov, Thomas Gleixner, Ingo Molnar, Dave Hansen,
x86, Rafael J. Wysocki, Peter Zijlstra, Adrian Hunter,
Kuppuswamy Sathyanarayanan, Elena Reshetova, Jun Nakajima,
Rick Edgecombe, Tom Lendacky, Sean Christopherson, Huang, Kai,
Ard Biesheuvel, Baoquan He, H. Peter Anvin, K. Y. Srinivasan,
Haiyang Zhang, kexec, linux-hyperv, linux-acpi, linux-coco,
linux-kernel, Tao Liu
In-Reply-To: <75cb309b-b05c-42a0-a3ca-de08fa1cae59@amd.com>
On Fri, May 31, 2024 at 12:34:49PM -0500, Kalra, Ashish wrote:
> SNP guest kexec patches are based on top of this patch-series and SNP guests
> also need this exclusive mem_enc_lock protection, so CC_ATTR_MEM_ENCRYPT
> makes sense to be used here.
Well, for the future, I'd encourage you to always send an Acked-by: you
or Reviewed-by: you as a reply to such patches so that it is clear that
such a change is desired.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* RE: [PATCH v6] Drivers: hv: Cosmetic changes for hv.c and balloon.c
From: Michael Kelley @ 2024-06-01 4:11 UTC (permalink / raw)
To: Aditya Nagesh, adityanagesh@microsoft.com, kys@microsoft.com,
haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <1717152521-6439-1-git-send-email-adityanagesh@linux.microsoft.com>
From: Aditya Nagesh <adityanagesh@linux.microsoft.com> Sent: Friday, May 31, 2024 3:49 AM
>
> Fix issues reported by checkpatch.pl script in hv.c and
> balloon.c
> - Remove unnecessary parentheses
> - Remove extra newlines
> - Remove extra spaces
> - Add spaces between comparison operators
> - Remove comparison with NULL in if statements
>
> No functional changes intended
>
> Signed-off-by: Aditya Nagesh <adityanagesh@linux.microsoft.com>
> Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---
> [V6]
> Fix build failure and unintended change after rebase
>
> [V5]
> Rebase to hyperv-fixes
>
> [V4]
> Fix Alignment issue and revert a line since 100 characters are allowed in a line
>
> [V3]
> Fix alignment issues in multiline function parameters.
>
> [V2]
> Change Subject from "Drivers: hv: Fix Issues reported by checkpatch.pl script"
> drivers/hv/hv.c | 37 ++++++++--------
> drivers/hv/hv_balloon.c | 98 +++++++++++++++--------------------------
> 2 files changed, 53 insertions(+), 82 deletions(-)
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
^ permalink raw reply
* [PATCH] x86/tdx: Enhance code generation for TDCALL and SEAMCALL wrappers
From: Kirill A. Shutemov @ 2024-06-02 11:54 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Dave Hansen, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H. Peter Anvin,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Josh Poimboeuf, Peter Zijlstra
Cc: linux-coco, linux-kernel, linux-hyperv, Kirill A. Shutemov
Sean observed that the compiler is generating inefficient code to clear
the tdx_module_args struct for TDCALL and SEAMCALL wrappers. The
compiler is generating numerous instructions at each call site to clear
the unused fields of the structure.
To address this issue, avoid using C99-initializer and instead
explicitly use string instructions to clear the struct.
With Clang, this change results in a savings of approximately 3K with my
configuration:
add/remove: 0/0 grow/shrink: 0/21 up/down: 0/-3187 (-3187)
With GCC, the savings are less significant at around 300 bytes:
add/remove: 0/0 grow/shrink: 3/22 up/down: 17/-313 (-296)
GCC tends to generate string instructions more frequently to clear the
struct.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Suggested-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Sean Christopherson <seanjc@google.com>
---
arch/x86/boot/compressed/tdx.c | 32 ++++---
arch/x86/coco/tdx/tdx-shared.c | 3 +-
arch/x86/coco/tdx/tdx.c | 150 +++++++++++++++++-------------
arch/x86/hyperv/ivm.c | 33 ++++---
arch/x86/include/asm/shared/tdx.h | 25 +++--
arch/x86/virt/vmx/tdx/tdx.c | 28 +++---
6 files changed, 155 insertions(+), 116 deletions(-)
diff --git a/arch/x86/boot/compressed/tdx.c b/arch/x86/boot/compressed/tdx.c
index 8451d6a1030c..a6784a9153e4 100644
--- a/arch/x86/boot/compressed/tdx.c
+++ b/arch/x86/boot/compressed/tdx.c
@@ -18,13 +18,14 @@ void __tdx_hypercall_failed(void)
static inline unsigned int tdx_io_in(int size, u16 port)
{
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = hcall_func(EXIT_REASON_IO_INSTRUCTION),
- .r12 = size,
- .r13 = 0,
- .r14 = port,
- };
+ struct tdx_module_args args;
+
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = hcall_func(EXIT_REASON_IO_INSTRUCTION);
+ args.r12 = size;
+ args.r13 = 0;
+ args.r14 = port;
if (__tdx_hypercall(&args))
return UINT_MAX;
@@ -34,14 +35,15 @@ static inline unsigned int tdx_io_in(int size, u16 port)
static inline void tdx_io_out(int size, u16 port, u32 value)
{
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = hcall_func(EXIT_REASON_IO_INSTRUCTION),
- .r12 = size,
- .r13 = 1,
- .r14 = port,
- .r15 = value,
- };
+ struct tdx_module_args args;
+
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = hcall_func(EXIT_REASON_IO_INSTRUCTION);
+ args.r12 = size;
+ args.r13 = 1;
+ args.r14 = port;
+ args.r15 = value;
__tdx_hypercall(&args);
}
diff --git a/arch/x86/coco/tdx/tdx-shared.c b/arch/x86/coco/tdx/tdx-shared.c
index 1655aa56a0a5..b8d1b3d940d2 100644
--- a/arch/x86/coco/tdx/tdx-shared.c
+++ b/arch/x86/coco/tdx/tdx-shared.c
@@ -5,7 +5,7 @@ static unsigned long try_accept_one(phys_addr_t start, unsigned long len,
enum pg_level pg_level)
{
unsigned long accept_size = page_level_size(pg_level);
- struct tdx_module_args args = {};
+ struct tdx_module_args args;
u8 page_size;
if (!IS_ALIGNED(start, accept_size))
@@ -34,6 +34,7 @@ static unsigned long try_accept_one(phys_addr_t start, unsigned long len,
return 0;
}
+ tdx_arg_init(&args);
args.rcx = start | page_size;
if (__tdcall(TDG_MEM_PAGE_ACCEPT, &args))
return 0;
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index c1cb90369915..8112b2910ca2 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -49,13 +49,14 @@ noinstr void __noreturn __tdx_hypercall_failed(void)
long tdx_kvm_hypercall(unsigned int nr, unsigned long p1, unsigned long p2,
unsigned long p3, unsigned long p4)
{
- struct tdx_module_args args = {
- .r10 = nr,
- .r11 = p1,
- .r12 = p2,
- .r13 = p3,
- .r14 = p4,
- };
+ struct tdx_module_args args;
+
+ tdx_arg_init(&args);
+ args.r10 = nr;
+ args.r11 = p1;
+ args.r12 = p2;
+ args.r13 = p3;
+ args.r14 = p4;
return __tdx_hypercall(&args);
}
@@ -89,13 +90,14 @@ static inline void tdcall(u64 fn, struct tdx_module_args *args)
*/
int tdx_mcall_get_report0(u8 *reportdata, u8 *tdreport)
{
- struct tdx_module_args args = {
- .rcx = virt_to_phys(tdreport),
- .rdx = virt_to_phys(reportdata),
- .r8 = TDREPORT_SUBTYPE_0,
- };
+ struct tdx_module_args args;
u64 ret;
+ tdx_arg_init(&args);
+ args.rcx = virt_to_phys(tdreport);
+ args.rdx = virt_to_phys(reportdata);
+ args.r8 = TDREPORT_SUBTYPE_0;
+
ret = __tdcall(TDG_MR_REPORT, &args);
if (ret) {
if (TDCALL_RETURN_CODE(ret) == TDCALL_INVALID_OPERAND)
@@ -130,11 +132,7 @@ EXPORT_SYMBOL_GPL(tdx_hcall_get_quote);
static void __noreturn tdx_panic(const char *msg)
{
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = TDVMCALL_REPORT_FATAL_ERROR,
- .r12 = 0, /* Error code: 0 is Panic */
- };
+ struct tdx_module_args args;
union {
/* Define register order according to the GHCI */
struct { u64 r14, r15, rbx, rdi, rsi, r8, r9, rdx; };
@@ -145,6 +143,11 @@ static void __noreturn tdx_panic(const char *msg)
/* VMM assumes '\0' in byte 65, if the message took all 64 bytes */
strtomem_pad(message.str, msg, '\0');
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = TDVMCALL_REPORT_FATAL_ERROR;
+ args.r12 = 0; /* Error code: 0 is Panic */
+
args.r8 = message.r8;
args.r9 = message.r9;
args.r14 = message.r14;
@@ -165,10 +168,12 @@ static void __noreturn tdx_panic(const char *msg)
static void tdx_parse_tdinfo(u64 *cc_mask)
{
- struct tdx_module_args args = {};
+ struct tdx_module_args args;
unsigned int gpa_width;
u64 td_attr;
+ tdx_arg_init(&args);
+
/*
* TDINFO TDX module call is used to get the TD execution environment
* information like GPA width, number of available vcpus, debug mode
@@ -252,11 +257,12 @@ static int ve_instr_len(struct ve_info *ve)
static u64 __cpuidle __halt(const bool irq_disabled)
{
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = hcall_func(EXIT_REASON_HLT),
- .r12 = irq_disabled,
- };
+ struct tdx_module_args args;
+
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = hcall_func(EXIT_REASON_HLT);
+ args.r12 = irq_disabled;
/*
* Emulate HLT operation via hypercall. More info about ABI
@@ -296,11 +302,12 @@ void __cpuidle tdx_safe_halt(void)
static int read_msr(struct pt_regs *regs, struct ve_info *ve)
{
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = hcall_func(EXIT_REASON_MSR_READ),
- .r12 = regs->cx,
- };
+ struct tdx_module_args args;
+
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = hcall_func(EXIT_REASON_MSR_READ);
+ args.r12 = regs->cx;
/*
* Emulate the MSR read via hypercall. More info about ABI
@@ -317,12 +324,13 @@ static int read_msr(struct pt_regs *regs, struct ve_info *ve)
static int write_msr(struct pt_regs *regs, struct ve_info *ve)
{
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = hcall_func(EXIT_REASON_MSR_WRITE),
- .r12 = regs->cx,
- .r13 = (u64)regs->dx << 32 | regs->ax,
- };
+ struct tdx_module_args args;
+
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = hcall_func(EXIT_REASON_MSR_WRITE);
+ args.r12 = regs->cx;
+ args.r13 = (u64)regs->dx << 32 | regs->ax;
/*
* Emulate the MSR write via hypercall. More info about ABI
@@ -337,12 +345,13 @@ static int write_msr(struct pt_regs *regs, struct ve_info *ve)
static int handle_cpuid(struct pt_regs *regs, struct ve_info *ve)
{
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = hcall_func(EXIT_REASON_CPUID),
- .r12 = regs->ax,
- .r13 = regs->cx,
- };
+ struct tdx_module_args args;
+
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = hcall_func(EXIT_REASON_CPUID);
+ args.r12 = regs->ax;
+ args.r13 = regs->cx;
/*
* Only allow VMM to control range reserved for hypervisor
@@ -379,14 +388,15 @@ static int handle_cpuid(struct pt_regs *regs, struct ve_info *ve)
static bool mmio_read(int size, unsigned long addr, unsigned long *val)
{
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = hcall_func(EXIT_REASON_EPT_VIOLATION),
- .r12 = size,
- .r13 = EPT_READ,
- .r14 = addr,
- .r15 = *val,
- };
+ struct tdx_module_args args;
+
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = hcall_func(EXIT_REASON_EPT_VIOLATION);
+ args.r12 = size;
+ args.r13 = EPT_READ;
+ args.r14 = addr;
+ args.r15 = *val;
if (__tdx_hypercall(&args))
return false;
@@ -508,16 +518,17 @@ static int handle_mmio(struct pt_regs *regs, struct ve_info *ve)
static bool handle_in(struct pt_regs *regs, int size, int port)
{
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = hcall_func(EXIT_REASON_IO_INSTRUCTION),
- .r12 = size,
- .r13 = PORT_READ,
- .r14 = port,
- };
+ struct tdx_module_args args;
u64 mask = GENMASK(BITS_PER_BYTE * size, 0);
bool success;
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = hcall_func(EXIT_REASON_IO_INSTRUCTION);
+ args.r12 = size;
+ args.r13 = PORT_READ;
+ args.r14 = port;
+
/*
* Emulate the I/O read via hypercall. More info about ABI can be found
* in TDX Guest-Host-Communication Interface (GHCI) section titled
@@ -602,7 +613,9 @@ __init bool tdx_early_handle_ve(struct pt_regs *regs)
void tdx_get_ve_info(struct ve_info *ve)
{
- struct tdx_module_args args = {};
+ struct tdx_module_args args;
+
+ tdx_arg_init(&args);
/*
* Called during #VE handling to retrieve the #VE info from the
@@ -745,14 +758,16 @@ static bool tdx_map_gpa(phys_addr_t start, phys_addr_t end, bool enc)
}
while (retry_count < max_retries_per_page) {
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = TDVMCALL_MAP_GPA,
- .r12 = start,
- .r13 = end - start };
-
+ struct tdx_module_args args;
u64 map_fail_paddr;
- u64 ret = __tdx_hypercall(&args);
+ u64 ret;
+
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = TDVMCALL_MAP_GPA;
+ args.r12 = start;
+ args.r13 = end - start;
+ ret = __tdx_hypercall(&args);
if (ret != TDVMCALL_STATUS_RETRY)
return !ret;
@@ -824,10 +839,8 @@ static bool tdx_enc_status_change_finish(unsigned long vaddr, int numpages,
void __init tdx_early_init(void)
{
- struct tdx_module_args args = {
- .rdx = TDCS_NOTIFY_ENABLES,
- .r9 = -1ULL,
- };
+ struct tdx_module_args args;
+
u64 cc_mask;
u32 eax, sig[3];
@@ -846,6 +859,9 @@ void __init tdx_early_init(void)
cc_set_mask(cc_mask);
/* Kernel does not use NOTIFY_ENABLES and does not need random #VEs */
+ tdx_arg_init(&args);
+ args.rdx = TDCS_NOTIFY_ENABLES;
+ args.r9 = -1ULL;
tdcall(TDG_VM_WR, &args);
/*
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index 768d73de0d09..4a49d09b23ad 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -385,27 +385,31 @@ static inline void hv_ghcb_msr_read(u64 msr, u64 *value) {}
#ifdef CONFIG_INTEL_TDX_GUEST
static void hv_tdx_msr_write(u64 msr, u64 val)
{
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = EXIT_REASON_MSR_WRITE,
- .r12 = msr,
- .r13 = val,
- };
+ struct tdx_module_args args;
+ u64 ret;
- u64 ret = __tdx_hypercall(&args);
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = EXIT_REASON_MSR_WRITE;
+ args.r12 = msr;
+ args.r13 = val;
+
+ ret = __tdx_hypercall(&args);
WARN_ONCE(ret, "Failed to emulate MSR write: %lld\n", ret);
}
static void hv_tdx_msr_read(u64 msr, u64 *val)
{
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = EXIT_REASON_MSR_READ,
- .r12 = msr,
- };
+ struct tdx_module_args args;
+ u64 ret;
- u64 ret = __tdx_hypercall(&args);
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = EXIT_REASON_MSR_READ;
+ args.r12 = msr;
+
+ ret = __tdx_hypercall(&args);
if (WARN_ONCE(ret, "Failed to emulate MSR read: %lld\n", ret))
*val = 0;
@@ -415,8 +419,9 @@ static void hv_tdx_msr_read(u64 msr, u64 *val)
u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2)
{
- struct tdx_module_args args = { };
+ struct tdx_module_args args;
+ tdx_arg_init(&args);
args.r10 = control;
args.rdx = param1;
args.r8 = param2;
diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
index fdfd41511b02..0519dd7cbb92 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -89,6 +89,14 @@ struct tdx_module_args {
u64 rsi;
};
+static __always_inline void tdx_arg_init(struct tdx_module_args *args)
+{
+ asm ("rep stosb"
+ : "+D" (args)
+ : "c" (sizeof(*args)), "a" (0)
+ : "memory");
+}
+
/* Used to communicate with the TDX module */
u64 __tdcall(u64 fn, struct tdx_module_args *args);
u64 __tdcall_ret(u64 fn, struct tdx_module_args *args);
@@ -103,14 +111,15 @@ u64 __tdx_hypercall(struct tdx_module_args *args);
*/
static inline u64 _tdx_hypercall(u64 fn, u64 r12, u64 r13, u64 r14, u64 r15)
{
- struct tdx_module_args args = {
- .r10 = TDX_HYPERCALL_STANDARD,
- .r11 = fn,
- .r12 = r12,
- .r13 = r13,
- .r14 = r14,
- .r15 = r15,
- };
+ struct tdx_module_args args;
+
+ tdx_arg_init(&args);
+ args.r10 = TDX_HYPERCALL_STANDARD;
+ args.r11 = fn;
+ args.r12 = r12;
+ args.r13 = r13;
+ args.r14 = r14;
+ args.r15 = r15;
return __tdx_hypercall(&args);
}
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 4e2b2e2ac9f9..50d1ff9d874f 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -103,7 +103,7 @@ static inline int sc_retry_prerr(sc_func_t func, sc_err_func_t err_func,
*/
static int try_init_module_global(void)
{
- struct tdx_module_args args = {};
+ struct tdx_module_args args;
static DEFINE_RAW_SPINLOCK(sysinit_lock);
static bool sysinit_done;
static int sysinit_ret;
@@ -115,6 +115,8 @@ static int try_init_module_global(void)
if (sysinit_done)
goto out;
+ tdx_arg_init(&args);
+
/* RCX is module attributes and all bits are reserved */
args.rcx = 0;
sysinit_ret = seamcall_prerr(TDH_SYS_INIT, &args);
@@ -146,7 +148,7 @@ static int try_init_module_global(void)
*/
int tdx_cpu_enable(void)
{
- struct tdx_module_args args = {};
+ struct tdx_module_args args;
int ret;
if (!boot_cpu_has(X86_FEATURE_TDX_HOST_PLATFORM))
@@ -166,6 +168,7 @@ int tdx_cpu_enable(void)
if (ret)
return ret;
+ tdx_arg_init(&args);
ret = seamcall_prerr(TDH_SYS_LP_INIT, &args);
if (ret)
return ret;
@@ -252,7 +255,7 @@ static int build_tdx_memlist(struct list_head *tmb_list)
static int read_sys_metadata_field(u64 field_id, u64 *data)
{
- struct tdx_module_args args = {};
+ struct tdx_module_args args;
int ret;
/*
@@ -260,6 +263,7 @@ static int read_sys_metadata_field(u64 field_id, u64 *data)
* - RDX (in): the field to read
* - R8 (out): the field data
*/
+ tdx_arg_init(&args);
args.rdx = field_id;
ret = seamcall_prerr_ret(TDH_SYS_RD, &args);
if (ret)
@@ -955,7 +959,7 @@ static int construct_tdmrs(struct list_head *tmb_list,
static int config_tdx_module(struct tdmr_info_list *tdmr_list, u64 global_keyid)
{
- struct tdx_module_args args = {};
+ struct tdx_module_args args;
u64 *tdmr_pa_array;
size_t array_sz;
int i, ret;
@@ -977,6 +981,7 @@ static int config_tdx_module(struct tdmr_info_list *tdmr_list, u64 global_keyid)
for (i = 0; i < tdmr_list->nr_consumed_tdmrs; i++)
tdmr_pa_array[i] = __pa(tdmr_entry(tdmr_list, i));
+ tdx_arg_init(&args);
args.rcx = __pa(tdmr_pa_array);
args.rdx = tdmr_list->nr_consumed_tdmrs;
args.r8 = global_keyid;
@@ -990,8 +995,9 @@ static int config_tdx_module(struct tdmr_info_list *tdmr_list, u64 global_keyid)
static int do_global_key_config(void *unused)
{
- struct tdx_module_args args = {};
+ struct tdx_module_args args;
+ tdx_arg_init(&args);
return seamcall_prerr(TDH_SYS_KEY_CONFIG, &args);
}
@@ -1056,11 +1062,11 @@ static int init_tdmr(struct tdmr_info *tdmr)
* TDMR in each call.
*/
do {
- struct tdx_module_args args = {
- .rcx = tdmr->base,
- };
+ struct tdx_module_args args;
int ret;
+ tdx_arg_init(&args);
+ args.rcx = tdmr->base;
ret = seamcall_prerr_ret(TDH_SYS_TDMR_INIT, &args);
if (ret)
return ret;
@@ -1284,15 +1290,15 @@ static bool is_pamt_page(unsigned long phys)
*/
static bool paddr_is_tdx_private(unsigned long phys)
{
- struct tdx_module_args args = {
- .rcx = phys & PAGE_MASK,
- };
+ struct tdx_module_args args;
u64 sret;
if (!boot_cpu_has(X86_FEATURE_TDX_HOST_PLATFORM))
return false;
/* Get page type from the TDX module */
+ tdx_arg_init(&args);
+ args.rcx = phys & PAGE_MASK;
sret = __seamcall_ret(TDH_PHYMEM_PAGE_RDMD, &args);
/*
--
2.43.0
^ permalink raw reply related
* [PATCHv11.1 10/19] x86/mm: Add callbacks to prepare encrypted memory for kexec
From: Kirill A. Shutemov @ 2024-06-02 12:39 UTC (permalink / raw)
To: bp
Cc: adrian.hunter, ardb, ashish.kalra, bhe, dave.hansen,
elena.reshetova, haiyangz, hpa, jun.nakajima, kai.huang, kexec,
kirill.shutemov, kys, linux-acpi, linux-coco, linux-hyperv,
linux-kernel, ltao, mingo, nik.borisov, peterz, rafael,
rick.p.edgecombe, sathyanarayanan.kuppuswamy, seanjc, tglx,
thomas.lendacky, x86
In-Reply-To: <20240529104257.GIZlcGsTkJHVBblkrY@fat_crate.local>
AMD SEV and Intel TDX guests allocate shared buffers for performing I/O.
This is done by allocating pages normally from the buddy allocator and
then converting them to shared using set_memory_decrypted().
On kexec, the second kernel is unaware of which memory has been
converted in this manner. It only sees E820_TYPE_RAM. Accessing shared
memory as private is fatal.
Therefore, the memory state must be reset to its original state before
starting the new kernel with kexec.
The process of converting shared memory back to private occurs in two
steps:
- enc_kexec_begin() stops new conversions.
- enc_kexec_finish() unshares all existing shared memory, reverting it
back to private.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Tested-by: Tao Liu <ltao@redhat.com>
---
arch/x86/include/asm/x86_init.h | 9 +++++++++
arch/x86/kernel/crash.c | 12 ++++++++++++
arch/x86/kernel/reboot.c | 12 ++++++++++++
arch/x86/kernel/x86_init.c | 4 ++++
4 files changed, 37 insertions(+)
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 28ac3cb9b987..6cade48811cc 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -149,12 +149,21 @@ struct x86_init_acpi {
* @enc_status_change_finish Notify HV after the encryption status of a range is changed
* @enc_tlb_flush_required Returns true if a TLB flush is needed before changing page encryption status
* @enc_cache_flush_required Returns true if a cache flush is needed before changing page encryption status
+ * @enc_kexec_begin Begin the two-step process of conversion shared memory back
+ * to private. It stops the new conversions from being started
+ * and waits in-flight conversions to finish, if possible.
+ * @enc_kexec_finish Finish the two-step process of conversion shared memory to
+ * private. All memory is private after the call.
+ * It called with all CPUs but one shutdown and interrupts
+ * disabled.
*/
struct x86_guest {
int (*enc_status_change_prepare)(unsigned long vaddr, int npages, bool enc);
int (*enc_status_change_finish)(unsigned long vaddr, int npages, bool enc);
bool (*enc_tlb_flush_required)(bool enc);
bool (*enc_cache_flush_required)(void);
+ void (*enc_kexec_begin)(bool crash);
+ void (*enc_kexec_finish)(void);
};
/**
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index f06501445cd9..74f6305eb9ec 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -128,6 +128,18 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
#ifdef CONFIG_HPET_TIMER
hpet_disable();
#endif
+
+ /*
+ * Non-crash kexec calls enc_kexec_begin() while scheduling is still
+ * active. This allows the callback to wait until all in-flight
+ * shared<->private conversions are complete. In a crash scenario,
+ * enc_kexec_begin() get call after all but one CPU has been shut down
+ * and interrupts have been disabled. This only allows the callback to
+ * detect a race with the conversion and report it.
+ */
+ x86_platform.guest.enc_kexec_begin(true);
+ x86_platform.guest.enc_kexec_finish();
+
crash_save_cpu(regs, safe_smp_processor_id());
}
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index f3130f762784..097313147ad3 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -12,6 +12,7 @@
#include <linux/delay.h>
#include <linux/objtool.h>
#include <linux/pgtable.h>
+#include <linux/kexec.h>
#include <acpi/reboot.h>
#include <asm/io.h>
#include <asm/apic.h>
@@ -716,6 +717,14 @@ static void native_machine_emergency_restart(void)
void native_machine_shutdown(void)
{
+ /*
+ * Call enc_kexec_begin() while all CPUs are still active and
+ * interrupts are enabled. This will allow all in-flight memory
+ * conversions to finish cleanly.
+ */
+ if (kexec_in_progress)
+ x86_platform.guest.enc_kexec_begin(false);
+
/* Stop the cpus and apics */
#ifdef CONFIG_X86_IO_APIC
/*
@@ -752,6 +761,9 @@ void native_machine_shutdown(void)
#ifdef CONFIG_X86_64
x86_platform.iommu_shutdown();
#endif
+
+ if (kexec_in_progress)
+ x86_platform.guest.enc_kexec_finish();
}
static void __machine_emergency_restart(int emergency)
diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
index a7143bb7dd93..8a79fb505303 100644
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -138,6 +138,8 @@ static int enc_status_change_prepare_noop(unsigned long vaddr, int npages, bool
static int enc_status_change_finish_noop(unsigned long vaddr, int npages, bool enc) { return 0; }
static bool enc_tlb_flush_required_noop(bool enc) { return false; }
static bool enc_cache_flush_required_noop(void) { return false; }
+static void enc_kexec_begin_noop(bool crash) {}
+static void enc_kexec_finish_noop(void) {}
static bool is_private_mmio_noop(u64 addr) {return false; }
struct x86_platform_ops x86_platform __ro_after_init = {
@@ -161,6 +163,8 @@ struct x86_platform_ops x86_platform __ro_after_init = {
.enc_status_change_finish = enc_status_change_finish_noop,
.enc_tlb_flush_required = enc_tlb_flush_required_noop,
.enc_cache_flush_required = enc_cache_flush_required_noop,
+ .enc_kexec_begin = enc_kexec_begin_noop,
+ .enc_kexec_finish = enc_kexec_finish_noop,
},
};
--
2.43.0
^ permalink raw reply related
* Re: [PATCHv11.1 10/19] x86/mm: Add callbacks to prepare encrypted memory for kexec
From: Kirill A. Shutemov @ 2024-06-02 12:42 UTC (permalink / raw)
To: bp
Cc: adrian.hunter, ardb, ashish.kalra, bhe, dave.hansen,
elena.reshetova, haiyangz, hpa, jun.nakajima, kai.huang, kexec,
kys, linux-acpi, linux-coco, linux-hyperv, linux-kernel, ltao,
mingo, nik.borisov, peterz, rafael, rick.p.edgecombe,
sathyanarayanan.kuppuswamy, seanjc, tglx, thomas.lendacky, x86
In-Reply-To: <20240602123903.2121883-1-kirill.shutemov@linux.intel.com>
Please disregard this. I failed to fold changes :/
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* [PATCHv11.2 10/19] x86/mm: Add callbacks to prepare encrypted memory for kexec
From: Kirill A. Shutemov @ 2024-06-02 12:44 UTC (permalink / raw)
To: bp
Cc: adrian.hunter, ardb, ashish.kalra, bhe, dave.hansen,
elena.reshetova, haiyangz, hpa, jun.nakajima, kai.huang, kexec,
kirill.shutemov, kys, linux-acpi, linux-coco, linux-hyperv,
linux-kernel, ltao, mingo, nik.borisov, peterz, rafael,
rick.p.edgecombe, sathyanarayanan.kuppuswamy, seanjc, tglx,
thomas.lendacky, x86
In-Reply-To: <20240529104257.GIZlcGsTkJHVBblkrY@fat_crate.local>
AMD SEV and Intel TDX guests allocate shared buffers for performing I/O.
This is done by allocating pages normally from the buddy allocator and
then converting them to shared using set_memory_decrypted().
On kexec, the second kernel is unaware of which memory has been
converted in this manner. It only sees E820_TYPE_RAM. Accessing shared
memory as private is fatal.
Therefore, the memory state must be reset to its original state before
starting the new kernel with kexec.
The process of converting shared memory back to private occurs in two
steps:
- enc_kexec_begin() stops new conversions.
- enc_kexec_finish() unshares all existing shared memory, reverting it
back to private.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Tested-by: Tao Liu <ltao@redhat.com>
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
arch/x86/include/asm/x86_init.h | 12 ++++++++++++
arch/x86/kernel/crash.c | 12 ++++++++++++
arch/x86/kernel/reboot.c | 12 ++++++++++++
arch/x86/kernel/x86_init.c | 4 ++++
4 files changed, 40 insertions(+)
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 28ac3cb9b987..b0f313278967 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -149,12 +149,24 @@ struct x86_init_acpi {
* @enc_status_change_finish Notify HV after the encryption status of a range is changed
* @enc_tlb_flush_required Returns true if a TLB flush is needed before changing page encryption status
* @enc_cache_flush_required Returns true if a cache flush is needed before changing page encryption status
+ * @enc_kexec_begin Begin the two-step process of converting shared memory back
+ * to private. It stops the new conversions from being started
+ * and waits in-flight conversions to finish, if possible.
+ * The @crash parameter denotes whether the function is being
+ * called in the crash shutdown path.
+ * @enc_kexec_finish Finish the two-step process of converting shared memory to
+ * private. All memory is private after the call when
+ * the function returns.
+ * It is called on only one CPU while the others are shut down
+ * and with interrupts disabled.
*/
struct x86_guest {
int (*enc_status_change_prepare)(unsigned long vaddr, int npages, bool enc);
int (*enc_status_change_finish)(unsigned long vaddr, int npages, bool enc);
bool (*enc_tlb_flush_required)(bool enc);
bool (*enc_cache_flush_required)(void);
+ void (*enc_kexec_begin)(bool crash);
+ void (*enc_kexec_finish)(void);
};
/**
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index f06501445cd9..fc52ea80cdc8 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -128,6 +128,18 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
#ifdef CONFIG_HPET_TIMER
hpet_disable();
#endif
+
+ /*
+ * Non-crash kexec calls enc_kexec_begin() while scheduling is still
+ * active. This allows the callback to wait until all in-flight
+ * shared<->private conversions are complete. In a crash scenario,
+ * enc_kexec_begin() gets called after all but one CPU have been shut
+ * down and interrupts have been disabled. This allows the callback to
+ * detect a race with the conversion and report it.
+ */
+ x86_platform.guest.enc_kexec_begin(true);
+ x86_platform.guest.enc_kexec_finish();
+
crash_save_cpu(regs, safe_smp_processor_id());
}
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index f3130f762784..097313147ad3 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -12,6 +12,7 @@
#include <linux/delay.h>
#include <linux/objtool.h>
#include <linux/pgtable.h>
+#include <linux/kexec.h>
#include <acpi/reboot.h>
#include <asm/io.h>
#include <asm/apic.h>
@@ -716,6 +717,14 @@ static void native_machine_emergency_restart(void)
void native_machine_shutdown(void)
{
+ /*
+ * Call enc_kexec_begin() while all CPUs are still active and
+ * interrupts are enabled. This will allow all in-flight memory
+ * conversions to finish cleanly.
+ */
+ if (kexec_in_progress)
+ x86_platform.guest.enc_kexec_begin(false);
+
/* Stop the cpus and apics */
#ifdef CONFIG_X86_IO_APIC
/*
@@ -752,6 +761,9 @@ void native_machine_shutdown(void)
#ifdef CONFIG_X86_64
x86_platform.iommu_shutdown();
#endif
+
+ if (kexec_in_progress)
+ x86_platform.guest.enc_kexec_finish();
}
static void __machine_emergency_restart(int emergency)
diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
index a7143bb7dd93..8a79fb505303 100644
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -138,6 +138,8 @@ static int enc_status_change_prepare_noop(unsigned long vaddr, int npages, bool
static int enc_status_change_finish_noop(unsigned long vaddr, int npages, bool enc) { return 0; }
static bool enc_tlb_flush_required_noop(bool enc) { return false; }
static bool enc_cache_flush_required_noop(void) { return false; }
+static void enc_kexec_begin_noop(bool crash) {}
+static void enc_kexec_finish_noop(void) {}
static bool is_private_mmio_noop(u64 addr) {return false; }
struct x86_platform_ops x86_platform __ro_after_init = {
@@ -161,6 +163,8 @@ struct x86_platform_ops x86_platform __ro_after_init = {
.enc_status_change_finish = enc_status_change_finish_noop,
.enc_tlb_flush_required = enc_tlb_flush_required_noop,
.enc_cache_flush_required = enc_cache_flush_required_noop,
+ .enc_kexec_begin = enc_kexec_begin_noop,
+ .enc_kexec_finish = enc_kexec_finish_noop,
},
};
--
2.43.0
^ permalink raw reply related
* Re: [PATCHv11 11/19] x86/tdx: Convert shared memory back to private on kexec
From: Kirill A. Shutemov @ 2024-06-02 14:20 UTC (permalink / raw)
To: Borislav Petkov
Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, x86, Rafael J. Wysocki,
Peter Zijlstra, Adrian Hunter, Kuppuswamy Sathyanarayanan,
Elena Reshetova, Jun Nakajima, Rick Edgecombe, Tom Lendacky,
Kalra, Ashish, Sean Christopherson, Huang, Kai, Ard Biesheuvel,
Baoquan He, H. Peter Anvin, K. Y. Srinivasan, Haiyang Zhang,
kexec, linux-hyperv, linux-acpi, linux-coco, linux-kernel,
Tao Liu
In-Reply-To: <20240531151442.GMZlnpYkDCRlg1_YS0@fat_crate.local>
On Fri, May 31, 2024 at 05:14:42PM +0200, Borislav Petkov wrote:
> On Tue, May 28, 2024 at 12:55:14PM +0300, Kirill A. Shutemov wrote:
> > +static void tdx_kexec_finish(void)
> > +{
> > + unsigned long addr, end;
> > + long found = 0, shared;
> > +
> > + lockdep_assert_irqs_disabled();
> > +
> > + addr = PAGE_OFFSET;
> > + end = PAGE_OFFSET + get_max_mapped();
> > +
> > + while (addr < end) {
> > + unsigned long size;
> > + unsigned int level;
> > + pte_t *pte;
> > +
> > + pte = lookup_address(addr, &level);
> > + size = page_level_size(level);
> > +
> > + if (pte && pte_decrypted(*pte)) {
> > + int pages = size / PAGE_SIZE;
> > +
> > + /*
> > + * Touching memory with shared bit set triggers implicit
> > + * conversion to shared.
> > + *
> > + * Make sure nobody touches the shared range from
> > + * now on.
> > + */
> > + set_pte(pte, __pte(0));
> > +
>
> Format the below into a comment here:
>
> /*
>
> The only thing one can do at this point on failure is panic. It is
> reasonable to proceed, especially for the crash case because the
> kexec-ed kernel is using a different page table so there won't be
> a mismatch between shared/private marking of the page so it doesn't
> matter.
Page tables would not make a difference here. We will switch to identity
mappings soon. And kexec-ed kernel will build new page tables from
scratch.
I will drop the part after "It is reasonable to proceed".
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* [PATCHv11.1 11/19] x86/tdx: Convert shared memory back to private on kexec
From: Kirill A. Shutemov @ 2024-06-02 14:23 UTC (permalink / raw)
To: bp
Cc: adrian.hunter, ardb, ashish.kalra, bhe, dave.hansen,
elena.reshetova, haiyangz, hpa, jun.nakajima, kai.huang, kexec,
kirill.shutemov, kys, linux-acpi, linux-coco, linux-hyperv,
linux-kernel, ltao, mingo, peterz, rafael, rick.p.edgecombe,
sathyanarayanan.kuppuswamy, seanjc, tglx, thomas.lendacky, x86
In-Reply-To: <20240531151442.GMZlnpYkDCRlg1_YS0@fat_crate.local>
TDX guests allocate shared buffers to perform I/O. It is done by
allocating pages normally from the buddy allocator and converting them
to shared with set_memory_decrypted().
The second, kexec-ed kernel has no idea what memory is converted this
way. It only sees E820_TYPE_RAM.
Accessing shared memory via private mapping is fatal. It leads to
unrecoverable TD exit.
On kexec walk direct mapping and convert all shared memory back to
private. It makes all RAM private again and second kernel may use it
normally.
The conversion occurs in two steps: stopping new conversions and
unsharing all memory. In the case of normal kexec, the stopping of
conversions takes place while scheduling is still functioning. This
allows for waiting until any ongoing conversions are finished. The
second step is carried out when all CPUs except one are inactive and
interrupts are disabled. This prevents any conflicts with code that may
access shared memory.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Tested-by: Tao Liu <ltao@redhat.com>
---
arch/x86/coco/tdx/tdx.c | 90 +++++++++++++++++++++++++++++++
arch/x86/include/asm/pgtable.h | 5 ++
arch/x86/include/asm/set_memory.h | 3 ++
arch/x86/mm/pat/set_memory.c | 41 ++++++++++++--
4 files changed, 136 insertions(+), 3 deletions(-)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 979891e97d83..afd71bc6eb02 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -7,6 +7,7 @@
#include <linux/cpufeature.h>
#include <linux/export.h>
#include <linux/io.h>
+#include <linux/kexec.h>
#include <asm/coco.h>
#include <asm/tdx.h>
#include <asm/vmx.h>
@@ -14,6 +15,7 @@
#include <asm/insn.h>
#include <asm/insn-eval.h>
#include <asm/pgtable.h>
+#include <asm/set_memory.h>
/* MMIO direction */
#define EPT_READ 0
@@ -831,6 +833,91 @@ static int tdx_enc_status_change_finish(unsigned long vaddr, int numpages,
return 0;
}
+/* Stop new private<->shared conversions */
+static void tdx_kexec_begin(bool crash)
+{
+ /*
+ * Crash kernel reaches here with interrupts disabled: can't wait for
+ * conversions to finish.
+ *
+ * If race happened, just report and proceed.
+ */
+ if (!set_memory_enc_stop_conversion(!crash))
+ pr_warn("Failed to stop shared<->private conversions\n");
+}
+
+/* Walk direct mapping and convert all shared memory back to private */
+static void tdx_kexec_finish(void)
+{
+ unsigned long addr, end;
+ long found = 0, shared;
+
+ lockdep_assert_irqs_disabled();
+
+ addr = PAGE_OFFSET;
+ end = PAGE_OFFSET + get_max_mapped();
+
+ while (addr < end) {
+ unsigned long size;
+ unsigned int level;
+ pte_t *pte;
+
+ pte = lookup_address(addr, &level);
+ size = page_level_size(level);
+
+ if (pte && pte_decrypted(*pte)) {
+ int pages = size / PAGE_SIZE;
+
+ /*
+ * Touching memory with shared bit set triggers implicit
+ * conversion to shared.
+ *
+ * Make sure nobody touches the shared range from
+ * now on.
+ */
+ set_pte(pte, __pte(0));
+
+ /*
+ * The only thing one can do at this point on failure
+ * is panic. It is reasonable to proceed.
+ *
+ * Also, even if the failure is real and the page cannot
+ * be touched as private, the kdump kernel will boot
+ * fine as it uses pre-reserved memory. What happens
+ * next depends on what the dumping process does and
+ * there's a reasonable chance to produce useful dump
+ * on crash.
+ *
+ * Regardless, the print leaves a trace in the log to
+ * give a clue for debug.
+ *
+ * One possible reason for the failure is if kdump raced
+ * with memory conversion. In this case shared bit in
+ * page table got set (or not cleared) during
+ * shared<->private conversion, but the page is actually
+ * private. So this failure is not going to affect the
+ * kexec'ed kernel.
+ */
+ if (!tdx_enc_status_changed(addr, pages, true)) {
+ pr_err("Failed to unshare range %#lx-%#lx\n",
+ addr, addr + size);
+ }
+
+ found += pages;
+ }
+
+ addr += size;
+ }
+
+ __flush_tlb_all();
+
+ shared = atomic_long_read(&nr_shared);
+ if (shared != found) {
+ pr_err("shared page accounting is off\n");
+ pr_err("nr_shared = %ld, nr_found = %ld\n", shared, found);
+ }
+}
+
void __init tdx_early_init(void)
{
struct tdx_module_args args = {
@@ -890,6 +977,9 @@ void __init tdx_early_init(void)
x86_platform.guest.enc_cache_flush_required = tdx_cache_flush_required;
x86_platform.guest.enc_tlb_flush_required = tdx_tlb_flush_required;
+ x86_platform.guest.enc_kexec_begin = tdx_kexec_begin;
+ x86_platform.guest.enc_kexec_finish = tdx_kexec_finish;
+
/*
* TDX intercepts the RDMSR to read the X2APIC ID in the parallel
* bringup low level code. That raises #VE which cannot be handled
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 65b8e5bb902c..e39311a89bf4 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -140,6 +140,11 @@ static inline int pte_young(pte_t pte)
return pte_flags(pte) & _PAGE_ACCESSED;
}
+static inline bool pte_decrypted(pte_t pte)
+{
+ return cc_mkdec(pte_val(pte)) == pte_val(pte);
+}
+
#define pmd_dirty pmd_dirty
static inline bool pmd_dirty(pmd_t pmd)
{
diff --git a/arch/x86/include/asm/set_memory.h b/arch/x86/include/asm/set_memory.h
index 9aee31862b4a..d490db38db9e 100644
--- a/arch/x86/include/asm/set_memory.h
+++ b/arch/x86/include/asm/set_memory.h
@@ -49,8 +49,11 @@ int set_memory_wb(unsigned long addr, int numpages);
int set_memory_np(unsigned long addr, int numpages);
int set_memory_p(unsigned long addr, int numpages);
int set_memory_4k(unsigned long addr, int numpages);
+
+bool set_memory_enc_stop_conversion(bool wait);
int set_memory_encrypted(unsigned long addr, int numpages);
int set_memory_decrypted(unsigned long addr, int numpages);
+
int set_memory_np_noalias(unsigned long addr, int numpages);
int set_memory_nonglobal(unsigned long addr, int numpages);
int set_memory_global(unsigned long addr, int numpages);
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index a7a7a6c6a3fb..2a548b65ef5f 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -2227,12 +2227,47 @@ static int __set_memory_enc_pgtable(unsigned long addr, int numpages, bool enc)
return ret;
}
+/*
+ * The lock serializes conversions between private and shared memory.
+ *
+ * It is taken for read on conversion. A write lock guarantees that no
+ * concurrent conversions are in progress.
+ */
+static DECLARE_RWSEM(mem_enc_lock);
+
+/*
+ * Stop new private<->shared conversions.
+ *
+ * Taking the exclusive mem_enc_lock waits for in-flight conversions to complete.
+ * The lock is not released to prevent new conversions from being started.
+ *
+ * If sleep is not allowed, as in a crash scenario, try to take the lock.
+ * Failure indicates that there is a race with the conversion.
+ */
+bool set_memory_enc_stop_conversion(bool wait)
+{
+ if (!wait)
+ return down_write_trylock(&mem_enc_lock);
+
+ down_write(&mem_enc_lock);
+
+ return true;
+}
+
static int __set_memory_enc_dec(unsigned long addr, int numpages, bool enc)
{
- if (cc_platform_has(CC_ATTR_MEM_ENCRYPT))
- return __set_memory_enc_pgtable(addr, numpages, enc);
+ int ret = 0;
- return 0;
+ if (cc_platform_has(CC_ATTR_MEM_ENCRYPT)) {
+ if (!down_read_trylock(&mem_enc_lock))
+ return -EBUSY;
+
+ ret = __set_memory_enc_pgtable(addr, numpages, enc);
+
+ up_read(&mem_enc_lock);
+ }
+
+ return ret;
}
int set_memory_encrypted(unsigned long addr, int numpages)
--
2.43.0
^ permalink raw reply related
* Re: [PATCHv11.1 11/19] x86/tdx: Convert shared memory back to private on kexec
From: Borislav Petkov @ 2024-06-03 8:37 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: adrian.hunter, ardb, ashish.kalra, bhe, dave.hansen,
elena.reshetova, haiyangz, hpa, jun.nakajima, kai.huang, kexec,
kys, linux-acpi, linux-coco, linux-hyperv, linux-kernel, ltao,
mingo, peterz, rafael, rick.p.edgecombe,
sathyanarayanan.kuppuswamy, seanjc, tglx, thomas.lendacky, x86
In-Reply-To: <20240602142303.3263551-1-kirill.shutemov@linux.intel.com>
On Sun, Jun 02, 2024 at 05:23:03PM +0300, Kirill A. Shutemov wrote:
> + /*
> + * The only thing one can do at this point on failure
> + * is panic. It is reasonable to proceed.
It makes even less sense now: panic() means "all stops and we die" and
you say it is reasonable to proceed.
I'm confused.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCHv11 18/19] x86/acpi: Add support for CPU offlining for ACPI MADT wakeup method
From: Borislav Petkov @ 2024-06-03 8:39 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Thomas Gleixner, Ingo Molnar, Dave Hansen, x86, Rafael J. Wysocki,
Peter Zijlstra, Adrian Hunter, Kuppuswamy Sathyanarayanan,
Elena Reshetova, Jun Nakajima, Rick Edgecombe, Tom Lendacky,
Kalra, Ashish, Sean Christopherson, Huang, Kai, Ard Biesheuvel,
Baoquan He, H. Peter Anvin, K. Y. Srinivasan, Haiyang Zhang,
kexec, linux-hyperv, linux-acpi, linux-coco, linux-kernel,
Tao Liu
In-Reply-To: <20240528095522.509667-19-kirill.shutemov@linux.intel.com>
On Tue, May 28, 2024 at 12:55:21PM +0300, Kirill A. Shutemov wrote:
> MADT Multiprocessor Wakeup structure version 1 brings support of CPU
s/of /for /
> offlining: BIOS provides a reset vector where the CPU has to jump to
> for offlining itself. The new TEST mailbox command can be used to test
> whether the CPU offlined itself which means the BIOS has control over
> the CPU and can online it again via the ACPI MADT wakeup method.
>
> Add CPU offling support for the ACPI MADT wakeup method by implementing
Unknown word [offling] in commit message.
Please introduce a spellchecker into your patch creation workflow.
> custom cpu_die(), play_dead() and stop_this_cpu() SMP operations.
>
> CPU offlining makes is possible to hand over secondary CPUs over kexec,
s/is /it /
> not limiting the second kernel to a single CPU.
...
> +/*
> + * Make sure asm_acpi_mp_play_dead() is present in the identity mapping at
> + * the same place as in the kernel page tables. asm_acpi_mp_play_dead() switches
> + * to the identity mapping and the function has be present at the same spot in
> + * the virtual address space before and after switching page tables.
> + */
> +static int __init init_transition_pgtable(pgd_t *pgd)
This looks like a generic helper which should be in set_memory.c. And
looking at that file, there's populate_pgd() which does pretty much the
same thing, if I squint real hard.
Let's tone down the duplication.
> +{
> + pgprot_t prot = PAGE_KERNEL_EXEC_NOENC;
> + unsigned long vaddr, paddr;
> + p4d_t *p4d;
> + pud_t *pud;
> + pmd_t *pmd;
> + pte_t *pte;
> +
> + vaddr = (unsigned long)asm_acpi_mp_play_dead;
> + pgd += pgd_index(vaddr);
> + if (!pgd_present(*pgd)) {
> + p4d = (p4d_t *)alloc_pgt_page(NULL);
> + if (!p4d)
> + return -ENOMEM;
> + set_pgd(pgd, __pgd(__pa(p4d) | _KERNPG_TABLE));
> + }
> + p4d = p4d_offset(pgd, vaddr);
> + if (!p4d_present(*p4d)) {
> + pud = (pud_t *)alloc_pgt_page(NULL);
> + if (!pud)
> + return -ENOMEM;
> + set_p4d(p4d, __p4d(__pa(pud) | _KERNPG_TABLE));
> + }
> + pud = pud_offset(p4d, vaddr);
> + if (!pud_present(*pud)) {
> + pmd = (pmd_t *)alloc_pgt_page(NULL);
> + if (!pmd)
> + return -ENOMEM;
> + set_pud(pud, __pud(__pa(pmd) | _KERNPG_TABLE));
> + }
> + pmd = pmd_offset(pud, vaddr);
> + if (!pmd_present(*pmd)) {
> + pte = (pte_t *)alloc_pgt_page(NULL);
> + if (!pte)
> + return -ENOMEM;
> + set_pmd(pmd, __pmd(__pa(pte) | _KERNPG_TABLE));
> + }
> + pte = pte_offset_kernel(pmd, vaddr);
> +
> + paddr = __pa(vaddr);
> + set_pte(pte, pfn_pte(paddr >> PAGE_SHIFT, prot));
> +
> + return 0;
> +}
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH net-next v3] net: mana: Allow variable size indirection table
From: Leon Romanovsky @ 2024-06-03 8:41 UTC (permalink / raw)
To: Shradha Gupta
Cc: linux-hardening, netdev, linux-hyperv, linux-kernel, linux-rdma,
Colin Ian King, Ahmed Zaki, Pavan Chebbi, Souradeep Chakrabarti,
Konstantin Taranov, Kees Cook, Paolo Abeni, Jakub Kicinski,
Eric Dumazet, David S. Miller, Dexuan Cui, Wei Liu, Haiyang Zhang,
K. Y. Srinivasan, Jason Gunthorpe, Long Li, Shradha Gupta
In-Reply-To: <1717169861-15825-1-git-send-email-shradhagupta@linux.microsoft.com>
On Fri, May 31, 2024 at 08:37:41AM -0700, Shradha Gupta wrote:
> Allow variable size indirection table allocation in MANA instead
> of using a constant value MANA_INDIRECT_TABLE_SIZE.
> The size is now derived from the MANA_QUERY_VPORT_CONFIG and the
> indirection table is allocated dynamically.
>
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Reviewed-by: Dexuan Cui <decui@microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> Changes in v3:
> * Fixed the memory leak(save_table) in mana_set_rxfh()
>
> Changes in v2:
> * Rebased to latest net-next tree
> * Rearranged cleanup code in mana_probe_port to avoid extra operations
> ---
> drivers/infiniband/hw/mana/qp.c | 10 +--
> drivers/net/ethernet/microsoft/mana/mana_en.c | 68 ++++++++++++++++---
> .../ethernet/microsoft/mana/mana_ethtool.c | 27 +++++---
> include/net/mana/gdma.h | 4 +-
> include/net/mana/mana.h | 9 +--
> 5 files changed, 89 insertions(+), 29 deletions(-)
<...>
> +free_indir:
> + apc->indir_table_sz = 0;
> + kfree(apc->indir_table);
> + apc->indir_table = NULL;
> + kfree(apc->rxobj_table);
> + apc->rxobj_table = NULL;
> reset_apc:
> kfree(apc->rxqs);
> apc->rxqs = NULL;
> @@ -2897,6 +2936,7 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
> {
<...>
> @@ -2931,6 +2972,11 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
> }
>
> unregister_netdevice(ndev);
> + apc->indir_table_sz = 0;
> + kfree(apc->indir_table);
> + apc->indir_table = NULL;
> + kfree(apc->rxobj_table);
> + apc->rxobj_table = NULL;
Why do you need to NULLify here? Will apc is going to be accessible
after call to mana_remove? or port probe failure?
Thanks
^ permalink raw reply
* Re: [PATCH 9/9] mmc: Convert from tasklet to BH workqueue
From: Aubin Constans @ 2024-06-03 12:38 UTC (permalink / raw)
To: Allen Pais, linux-kernel
Cc: tj, keescook, vkoul, marcan, sven, florian.fainelli, rjui,
sbranden, paul, Eugeniy.Paltsev, manivannan.sadhasivam, vireshk,
Frank.Li, leoyang.li, zw, wangzhou1, haijie1, shawnguo, s.hauer,
sean.wang, matthias.bgg, angelogioacchino.delregno, afaerber,
logang, daniel, haojian.zhuang, robert.jarzmik, andersson,
konrad.dybcio, orsonzhai, baolin.wang, zhang.lyra,
patrice.chotard, linus.walleij, wens, jernej.skrabec,
peter.ujfalusi, kys, haiyangz, wei.liu, decui, jassisinghbrar,
mchehab, maintainers, ulf.hansson, manuel.lauss, mirq-linux,
jh80.chung, oakad, hayashi.kunihiko, mhiramat, brucechang,
HaraldWelte, pierre, duncan.sands, stern, oneukum,
openipmi-developer, dmaengine, asahi, linux-arm-kernel,
linux-rpi-kernel, linux-mips, imx, linuxppc-dev, linux-mediatek,
linux-actions, linux-arm-msm, linux-riscv, linux-sunxi,
linux-tegra, linux-hyperv, linux-rdma, linux-media, linux-mmc,
linux-omap, linux-renesas-soc, linux-s390, netdev, linux-usb
In-Reply-To: <20240327160314.9982-10-apais@linux.microsoft.com>
On 27/03/2024 17:03, Allen Pais wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> The only generic interface to execute asynchronously in the BH context is
> tasklet; however, it's marked deprecated and has some design flaws. To
> replace tasklets, BH workqueue support was recently added. A BH workqueue
> behaves similarly to regular workqueues except that the queued work items
> are executed in the BH context.
>
> This patch converts drivers/infiniband/* from tasklet to BH workqueue.
>
> Based on the work done by Tejun Heo <tj@kernel.org>
> Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
>
> Signed-off-by: Allen Pais <allen.lkml@gmail.com>
> ---
> drivers/mmc/host/atmel-mci.c | 35 ++++-----
[...]
For atmel-mci, judging from a few simple tests, performance is preserved.
E.g. writing to a SD Card on the SAMA5D3-Xplained board:
time dd if=/dev/zero of=/opt/_del_me bs=4k count=64k
Base 6.9.0 : 0.07user 5.05system 0:18.92elapsed 27%CPU
Patched 6.9.0+: 0.12user 4.92system 0:18.76elapsed 26%CPU
However, please resolve what checkpatch is complaining about:
scripts/checkpatch.pl --strict
PATCH-9-9-mmc-Convert-from-tasklet-to-BH-workqueue.mbox
WARNING: please, no space before tabs
#72: FILE: drivers/mmc/host/atmel-mci.c:367:
+^Istruct work_struct ^Iwork;$
Same as discussions on the USB patch[1] and others in this series, I am
also in favour of "workqueue" or similar in the comments, rather than
just "work".
Apart from that:
Tested-by: Aubin Constans <aubin.constans@microchip.com>
Acked-by: Aubin Constans <aubin.constans@microchip.com>
Thanks.
[1]:
https://lore.kernel.org/linux-mmc/CAOMdWSLipPfm3OZTpjZz4uF4M+E_8QAoTeMcKBXawLnkTQx6Jg@mail.gmail.com/
^ permalink raw reply
* Re: [PATCH] x86/tdx: Enhance code generation for TDCALL and SEAMCALL wrappers
From: Dave Hansen @ 2024-06-03 13:37 UTC (permalink / raw)
To: Kirill A. Shutemov, Sean Christopherson, Paolo Bonzini,
Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Josh Poimboeuf, Peter Zijlstra
Cc: linux-coco, linux-kernel, linux-hyperv
In-Reply-To: <20240602115444.1863364-1-kirill.shutemov@linux.intel.com>
On 6/2/24 04:54, Kirill A. Shutemov wrote:
> Sean observed that the compiler is generating inefficient code to clear
> the tdx_module_args struct for TDCALL and SEAMCALL wrappers. The
> compiler is generating numerous instructions at each call site to clear
> the unused fields of the structure.
>
> To address this issue, avoid using C99-initializer and instead
> explicitly use string instructions to clear the struct.
>
> With Clang, this change results in a savings of approximately 3K with my
> configuration:
>
> add/remove: 0/0 grow/shrink: 0/21 up/down: 0/-3187 (-3187)
>
> With GCC, the savings are less significant at around 300 bytes:
>
> add/remove: 0/0 grow/shrink: 3/22 up/down: 17/-313 (-296)
>
> GCC tends to generate string instructions more frequently to clear the
> struct.
<shrug>
I don't think moving away from perfectly normal C struct initialization
is worth it for 300 bytes of text in couple of slow paths.
If someone out there is using clang, is confident that it is doing the
right thing and not just being silly, _and_ is horribly bothered by its
code generation, then please speak up.
> +static __always_inline void tdx_arg_init(struct tdx_module_args *args)
> +{
> + asm ("rep stosb"
> + : "+D" (args)
> + : "c" (sizeof(*args)), "a" (0)
> + : "memory");
> +}
The inline assembly also has the side-effect of tripping up the
compiler. The compiler can't optimize across these at all and it
probably has the effect of bloating the code.
Oh, and if we're going to leave this weirdo initialization idiom for
TDX, it needs to be well commented:
/*
* Using normal " = {};" to initialize tdx_module_args results in
* bloated hard-to-read assembly. Zero it using the most compact way
* available.
*/
Eh?
^ permalink raw reply
* Re: [PATCH 9/9] mmc: Convert from tasklet to BH workqueue
From: Allen Pais @ 2024-06-03 17:25 UTC (permalink / raw)
To: Aubin Constans
Cc: Linux Kernel Mailing List, Tejun Heo, Kees Cook, Vinod Koul,
marcan, sven, florian.fainelli, Ray Jui, Scott Branden,
Paul Cercueil, Eugeniy.Paltsev, manivannan.sadhasivam,
Viresh Kumar, Frank.Li, Leo Li, zw, Zhou Wang, haijie1, Shawn Guo,
Sascha Hauer, Sean Wang, Matthias Brugger,
angelogioacchino.delregno, Andreas Färber, Logan Gunthorpe,
Daniel Mack, Haojian Zhuang, Robert Jarzmik, andersson,
konrad.dybcio, Orson Zhai, baolin.wang, Lyra Zhang,
Patrice CHOTARD, Linus Walleij, Chen-Yu Tsai, Jernej Škrabec,
peter.ujfalusi, kys, haiyangz, wei.liu, decui, jassisinghbrar,
mchehab, maintainers, ulf.hansson, manuel.lauss, mirq-linux,
jh80.chung, oakad, hayashi.kunihiko, mhiramat, brucechang,
HaraldWelte, pierre, duncan.sands, stern, oneukum,
openipmi-developer, dmaengine, asahi, linux-arm-kernel,
linux-rpi-kernel, linux-mips, imx, linuxppc-dev, linux-mediatek,
linux-actions, linux-arm-msm, linux-riscv, linux-sunxi,
linux-tegra, linux-hyperv, linux-rdma, linux-media, linux-mmc,
linux-omap, linux-renesas-soc, linux-s390, netdev, linux-usb
In-Reply-To: <7e618af0-51a7-4941-a386-0ac68c66d358@microchip.com>
> On Jun 3, 2024, at 5:38 AM, Aubin Constans <aubin.constans@microchip.com> wrote:
>
> On 27/03/2024 17:03, Allen Pais wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>> The only generic interface to execute asynchronously in the BH context is
>> tasklet; however, it's marked deprecated and has some design flaws. To
>> replace tasklets, BH workqueue support was recently added. A BH workqueue
>> behaves similarly to regular workqueues except that the queued work items
>> are executed in the BH context.
>> This patch converts drivers/infiniband/* from tasklet to BH workqueue.
>> Based on the work done by Tejun Heo <tj@kernel.org>
>> Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
>> Signed-off-by: Allen Pais <allen.lkml@gmail.com>
>> ---
>> drivers/mmc/host/atmel-mci.c | 35 ++++-----
> [...]
>
> For atmel-mci, judging from a few simple tests, performance is preserved.
> E.g. writing to a SD Card on the SAMA5D3-Xplained board:
> time dd if=/dev/zero of=/opt/_del_me bs=4k count=64k
>
> Base 6.9.0 : 0.07user 5.05system 0:18.92elapsed 27%CPU
> Patched 6.9.0+: 0.12user 4.92system 0:18.76elapsed 26%CPU
>
> However, please resolve what checkpatch is complaining about:
> scripts/checkpatch.pl --strict PATCH-9-9-mmc-Convert-from-tasklet-to-BH-workqueue.mbox
>
> WARNING: please, no space before tabs
> #72: FILE: drivers/mmc/host/atmel-mci.c:367:
> +^Istruct work_struct ^Iwork;$
>
> Same as discussions on the USB patch[1] and others in this series, I am also in favour of "workqueue" or similar in the comments, rather than just "work".
Will send out a new version.
Thank you very much for testing and providing your review.
- Allen
>
> Apart from that:
> Tested-by: Aubin Constans <aubin.constans@microchip.com>
> Acked-by: Aubin Constans <aubin.constans@microchip.com>
>
> Thanks.
>
> [1]: https://lore.kernel.org/linux-mmc/CAOMdWSLipPfm3OZTpjZz4uF4M+E_8QAoTeMcKBXawLnkTQx6Jg@mail.gmail.com/
^ permalink raw reply
* Re: [RFC PATCH v3 3/5] KVM: x86: Add notifications for Heki policy configuration and violation
From: Mickaël Salaün @ 2024-06-03 18:39 UTC (permalink / raw)
To: Sean Christopherson
Cc: Nicolas Saenz Julienne, Borislav Petkov, Dave Hansen,
H . Peter Anvin, Ingo Molnar, Kees Cook, Paolo Bonzini,
Thomas Gleixner, Vitaly Kuznetsov, Wanpeng Li, Rick P Edgecombe,
Alexander Graf, Angelina Vu, Anna Trikalinou, Chao Peng,
Forrest Yuan Yu, James Gowans, James Morris, John Andersen,
Madhavan T . Venkataraman, Marian Rotariu, Mihai Donțu,
Nicușor Cîțu, Thara Gopinath, Trilok Soni, Wei Liu,
Will Deacon, Yu Zhang, Ștefan Șicleru, dev, kvm,
linux-hardening, linux-hyperv, linux-kernel,
linux-security-module, qemu-devel, virtualization, x86, xen-devel
In-Reply-To: <ZkUb2IWj4Z9FziCb@google.com>
On Wed, May 15, 2024 at 01:32:24PM -0700, Sean Christopherson wrote:
> On Tue, May 14, 2024, Mickaël Salaün wrote:
> > On Fri, May 10, 2024 at 10:07:00AM +0000, Nicolas Saenz Julienne wrote:
> > > Development happens
> > > https://github.com/vianpl/{linux,qemu,kvm-unit-tests} and the vsm-next
> > > branch, but I'd advice against looking into it until we add some order
> > > to the rework. Regardless, feel free to get in touch.
> >
> > Thanks for the update.
> >
> > Could we schedule a PUCK meeting to synchronize and help each other?
> > What about June 12?
>
> June 12th works on my end.
Can you please send an invite?
Mickaël
^ permalink raw reply
* Re: [PATCHv11 05/19] x86/relocate_kernel: Use named labels for less confusion
From: H. Peter Anvin @ 2024-06-03 14:43 UTC (permalink / raw)
To: Nikolay Borisov, Kirill A. Shutemov, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86
Cc: Rafael J. Wysocki, Peter Zijlstra, Adrian Hunter,
Kuppuswamy Sathyanarayanan, Elena Reshetova, Jun Nakajima,
Rick Edgecombe, Tom Lendacky, Kalra, Ashish, Sean Christopherson,
Huang, Kai, Ard Biesheuvel, Baoquan He, K. Y. Srinivasan,
Haiyang Zhang, kexec, linux-hyperv, linux-acpi, linux-coco,
linux-kernel
In-Reply-To: <1e1d1aea-7346-4022-9f5f-402d171adfda@suse.com>
On 5/29/24 03:47, Nikolay Borisov wrote:
>>
>> diff --git a/arch/x86/kernel/relocate_kernel_64.S
>> b/arch/x86/kernel/relocate_kernel_64.S
>> index 56cab1bb25f5..085eef5c3904 100644
>> --- a/arch/x86/kernel/relocate_kernel_64.S
>> +++ b/arch/x86/kernel/relocate_kernel_64.S
>> @@ -148,9 +148,10 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
>> */
>> movl $X86_CR4_PAE, %eax
>> testq $X86_CR4_LA57, %r13
>> - jz 1f
>> + jz .Lno_la57
>> orl $X86_CR4_LA57, %eax
>> -1:
>> +.Lno_la57:
>> +
>> movq %rax, %cr4
>> jmp 1f
>
> That jmp 1f becomes redundant now as it simply jumps 1 line below.
>
Uh... am I the only person to notice that ALL that is needed here is:
andl $(X86_CR4_PAE|X86_CR4_LA57), %r13d
movq %r13, %rax
... since %r13 is dead afterwards, and PAE *will* have been set in %r13
already?
I don't believe that this specific jmp is actually needed -- there are
several more synchronizing jumps later -- but it doesn't hurt.
However, if the effort is for improving the readability, it might be
worthwhile to encapsulate the "jmp 1f; 1:" as a macro, e.g. "SYNC_CODE".
-hpa
^ permalink raw reply
* Re: [PATCHv11 05/19] x86/relocate_kernel: Use named labels for less confusion
From: H. Peter Anvin @ 2024-06-03 22:43 UTC (permalink / raw)
To: Nikolay Borisov, Kirill A. Shutemov, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86
Cc: Rafael J. Wysocki, Peter Zijlstra, Adrian Hunter,
Kuppuswamy Sathyanarayanan, Elena Reshetova, Jun Nakajima,
Rick Edgecombe, Tom Lendacky, Kalra, Ashish, Sean Christopherson,
Huang, Kai, Ard Biesheuvel, Baoquan He, K. Y. Srinivasan,
Haiyang Zhang, kexec, linux-hyperv, linux-acpi, linux-coco,
linux-kernel
In-Reply-To: <1e1d1aea-7346-4022-9f5f-402d171adfda@suse.com>
On 5/29/24 03:47, Nikolay Borisov wrote:
>>
>> diff --git a/arch/x86/kernel/relocate_kernel_64.S
>> b/arch/x86/kernel/relocate_kernel_64.S
>> index 56cab1bb25f5..085eef5c3904 100644
>> --- a/arch/x86/kernel/relocate_kernel_64.S
>> +++ b/arch/x86/kernel/relocate_kernel_64.S
>> @@ -148,9 +148,10 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
>> */
>> movl $X86_CR4_PAE, %eax
>> testq $X86_CR4_LA57, %r13
>> - jz 1f
>> + jz .Lno_la57
>> orl $X86_CR4_LA57, %eax
>> -1:
>> +.Lno_la57:
>> +
>> movq %rax, %cr4
>> jmp 1f
>
Sorry if this is a duplicate; something strange happened with my email.
If you are cleaning up this code anyway...
this whole piece of code can be simplified to:
and $(X86_CR4_PAE | X86_CR4_LA57), %r13d
mov %r13, %cr4
The PAE bit in %r13 is guaranteed to be set, and %r13 is dead after this.
-hpa
^ permalink raw reply
* Re: [RFC PATCH v3 3/5] KVM: x86: Add notifications for Heki policy configuration and violation
From: Sean Christopherson @ 2024-06-04 0:29 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Nicolas Saenz Julienne, Borislav Petkov, Dave Hansen,
H . Peter Anvin, Ingo Molnar, Kees Cook, Paolo Bonzini,
Thomas Gleixner, Vitaly Kuznetsov, Wanpeng Li, Rick P Edgecombe,
Alexander Graf, Angelina Vu, Anna Trikalinou, Chao Peng,
Forrest Yuan Yu, James Gowans, James Morris, John Andersen,
Madhavan T . Venkataraman, Marian Rotariu, Mihai Donțu,
Nicușor Cîțu, Thara Gopinath, Trilok Soni, Wei Liu,
Will Deacon, Yu Zhang, Ștefan Șicleru, dev, kvm,
linux-hardening, linux-hyperv, linux-kernel,
linux-security-module, qemu-devel, virtualization, x86, xen-devel
In-Reply-To: <20240514.OoPohLaejai6@digikod.net>
On Tue, May 14, 2024, Mickaël Salaün wrote:
> On Tue, May 07, 2024 at 09:16:06AM -0700, Sean Christopherson wrote:
> > On Tue, May 07, 2024, Mickaël Salaün wrote:
> > > If yes, that would indeed require a *lot* of work for something we're not
> > > sure will be accepted later on.
> >
> > Yes and no. The AWS folks are pursuing VSM support in KVM+QEMU, and SVSM support
> > is trending toward the paired VM+vCPU model. IMO, it's entirely feasible to
> > design KVM support such that much of the development load can be shared between
> > the projects. And having 2+ use cases for a feature (set) makes it _much_ more
> > likely that the feature(s) will be accepted.
> >
> > And similar to what Paolo said regarding HEKI not having a complete story, I
> > don't see a clear line of sight for landing host-defined policy enforcement, as
> > there are many open, non-trivial questions that need answers. I.e. upstreaming
> > HEKI in its current form is also far from a done deal, and isn't guaranteed to
> > be substantially less work when all is said and done.
>
> I'm not sure to understand why "Heki not having a complete story". The
> goal is the same as the current kernel self-protection mechanisms.
HEKI doesn't have a complete story for how it's going to play nice with kexec(),
emulated RESET, etc. The kernel's existing self-protection mechanisms Just Work
because the protections are automatically disabled/lost on such transitions.
They are obviously significant drawbacks to that behavior, but they are accepted
drawbacks, i.e. solving those problems isn't in scope (yet) for the kernel. And
the "failure" mode is also loss of hardening, not an unusable guest.
In other words, the kernel's hardening is firmly best effort at this time,
whereas HEKI likely needs to be much more than "best effort" in order to justify
the extra complexity. And that means having answers to the various interoperability
questions.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox