On 24.11.2023 11:30, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/p2m.h
> @@ -0,0 +1,105 @@
> +#ifndef __ASM_RISCV_P2M_H__
> +#define __ASM_RISCV_P2M_H__
> +
> +#include <asm/page-bits.h>
> +
> +#define paddr_bits PADDR_BITS
> +
> +/*
> + * List of possible type for each page in the p2m entry.
> + * The number of available bit per page in the pte for this purpose is 4
> bits.
> + * So it's possible to only have 16 fields. If we run out of value in the
> + * future, it's possible to use higher value for pseudo-type and don't store
> + * them in the p2m entry.
> + */
> +typedef enum {
> + p2m_invalid = 0, /* Nothing mapped here */
> + p2m_ram_rw, /* Normal read/write guest RAM */
> + p2m_ram_ro, /* Read-only; writes are silently dropped */
> + p2m_mmio_direct_dev,/* Read/write mapping of genuine Device MMIO area */
> + p2m_mmio_direct_nc, /* Read/write mapping of genuine MMIO area
> non-cacheable */
> + p2m_mmio_direct_c, /* Read/write mapping of genuine MMIO area cacheable
> */
> + p2m_map_foreign_rw, /* Read/write RAM pages from foreign domain */
> + p2m_map_foreign_ro, /* Read-only RAM pages from foreign domain */
> + p2m_grant_map_rw, /* Read/write grant mapping */
> + p2m_grant_map_ro, /* Read-only grant mapping */
> + /* The types below are only used to decide the page attribute in the P2M
> */
> + p2m_iommu_map_rw, /* Read/write iommu mapping */
> + p2m_iommu_map_ro, /* Read-only iommu mapping */
> + p2m_max_real_type, /* Types after this won't be store in the p2m */
> +} p2m_type_t;
As indicated before, I think you should have only those types here which
you're sure you'll need, or even just those which you really need right
away. I question in particular p2m_mmio_direct_*, which all look like
you simply took them from Arm, without regard as to applicability to
RISC-V. The fewer types you have here, the easier it is going to be to
tell what needs adding and what is already _properly_ supported.
> +#include <xen/p2m-common.h>
> +
> +static inline int get_page_and_type(struct page_info *page,
> + struct domain *domain,
> + unsigned long type)
> +{
> + BUG();
> + return 1;
> +}
Imo despite the BUG() any such stub would better return failure, just
like ...
> +/* Look up a GFN and take a reference count on the backing page. */
> +typedef unsigned int p2m_query_t;
> +#define P2M_ALLOC (1u<<0) /* Populate PoD and paged-out entries */
> +#define P2M_UNSHARE (1u<<1) /* Break CoW sharing */
> +
> +static inline struct page_info *get_page_from_gfn(
> + struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
> +{
> + BUG();
> + return NULL;
> +}
... you do here. May apply again further down.
> +static inline void memory_type_changed(struct domain *d)
> +{
> + BUG();
> +}
> +
> +
> +static inline int guest_physmap_mark_populate_on_demand(struct domain *d,
> unsigned long gfn,
> + unsigned int order)
> +{
> + BUG();
> + return 1;
> +}
> +
> +static inline int guest_physmap_add_entry(struct domain *d,
> + gfn_t gfn,
> + mfn_t mfn,
> + unsigned long page_order,
> + p2m_type_t t)
> +{
> + BUG();
> + return 1;
> +}
> +
> +/* Untyped version for RAM only, for compatibility */
> +static inline int __must_check
> +guest_physmap_add_page(struct domain *d, gfn_t gfn, mfn_t mfn,
> + unsigned int page_order)
> +{
> + return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
> +}
> +
> +static inline mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn)
> +{
> + BUG();
> + return _mfn(0);
> +}
> +
> +static inline bool arch_acquire_resource_check(struct domain *d)
> +{
> + /*
> + * The reference counting of foreign entries in set_foreign_p2m_entry()
> + * is supported on RISCV.
> + */
> + return true;
> +}
> +
> +static inline void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
> +{
> + /* Not supported on RISCV. */
> +}
> +
> +#endif /* __ASM_RISCV_P2M_H__ */
> \ No newline at end of file
This wants taking care of.
Jan
