Hi Jan,
On 27/03/2020 13:50, Jan Beulich wrote:
On 22.03.2020 17:14, jul...@xen.org wrote:
--- a/xen/arch/x86/hvm/domain.c
+++ b/xen/arch/x86/hvm/domain.c
@@ -296,8 +296,10 @@ int arch_set_info_hvm_guest(struct vcpu *v, const
vcpu_hvm_context_t *ctx)
if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) )
{
/* Shadow-mode CR3 change. Check PDBR and update refcounts. */
- struct page_info *page = get_page_from_gfn(v->domain,
- v->arch.hvm.guest_cr[3] >> PAGE_SHIFT,
+ struct page_info *page;
+
+ page = get_page_from_gfn(v->domain,
+ gaddr_to_gfn(v->arch.hvm.guest_cr[3]),
My earlier comment on this remains - I thing this conversion makes
the problem this expression has more hidden than with the shift.
This would better use a gfn_from_cr3() helper (or whatever it'll
be that it gets named). Same elsewhere in this patch then.
I will have a closer look the *cr3 helpers and reply with some suggestions.
@@ -2363,7 +2364,7 @@ int hvm_set_cr3(unsigned long value, bool may_defer)
{
/* Shadow-mode CR3 change. Check PDBR and update refcounts. */
HVM_DBG_LOG(DBG_LEVEL_VMMU, "CR3 value = %lx", value);
- page = get_page_from_gfn(v->domain, value >> PAGE_SHIFT,
+ page = get_page_from_gfn(v->domain, cr3_to_gfn(value),
Oh, seeing this I recall Paul did point out the above already.
@@ -508,7 +508,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control
*init_control)
{
struct domain *d = current->domain;
uint32_t vcpu_id;
- uint64_t gfn;
+ gfn_t gfn;
uint32_t offset;
struct vcpu *v;
int rc;
@@ -516,7 +516,7 @@ int evtchn_fifo_init_control(struct evtchn_init_control
*init_control)
init_control->link_bits = EVTCHN_FIFO_LINK_BITS;
vcpu_id = init_control->vcpu;
- gfn = init_control->control_gfn;
+ gfn = _gfn(init_control->control_gfn);
There's silent truncation here now for Arm32, afaict. Are we really
okay with this?
Well, the truncation was already silently happening as we call
get_page_from_gfn() in map_guest_page(). So it is not worse than the
current situation.
Although, there are a slight advantage with the new code as you can more
easily spot potential truncation. Indeed, you could add some type check
in _gfn().
Cheers,
--
Julien Grall